* [PATCH v2 17/35] net/wireless: Use kmemdup rather than duplicating its implementation
From: Fuqian Huang @ 2019-07-03 16:29 UTC (permalink / raw)
Cc: Kalle Valo, David S . Miller, Solomon Peachy, linux-wireless,
netdev, linux-kernel, Fuqian Huang
kmemdup is introduced to duplicate a region of memory in a neat way.
Rather than kmalloc/kzalloc + memcpy, which the programmer needs to
write the size twice (sometimes lead to mistakes), kmemdup improves
readability, leads to smaller code and also reduce the chances of mistakes.
Suggestion to use kmemdup rather than using kmalloc/kzalloc + memcpy.
Signed-off-by: Fuqian Huang <huangfq.daxian@gmail.com>
---
Changes in v2:
- Fix a typo in commit message (memset -> memcpy)
drivers/net/wireless/ath/ath6kl/wmi.c | 6 ++----
drivers/net/wireless/st/cw1200/queue.c | 3 +--
drivers/net/wireless/ti/wlcore/main.c | 3 +--
3 files changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/net/wireless/ath/ath6kl/wmi.c b/drivers/net/wireless/ath/ath6kl/wmi.c
index 68854c45d0a4..7452a0f587fe 100644
--- a/drivers/net/wireless/ath/ath6kl/wmi.c
+++ b/drivers/net/wireless/ath/ath6kl/wmi.c
@@ -3643,7 +3643,7 @@ static int ath6kl_wmi_send_action_cmd(struct wmi *wmi, u8 if_idx, u32 id,
if (wait)
return -EINVAL; /* Offload for wait not supported */
- buf = kmalloc(data_len, GFP_KERNEL);
+ buf = kmemdup(data, data_len, GFP_KERNEL);
if (!buf)
return -ENOMEM;
@@ -3654,7 +3654,6 @@ static int ath6kl_wmi_send_action_cmd(struct wmi *wmi, u8 if_idx, u32 id,
}
kfree(wmi->last_mgmt_tx_frame);
- memcpy(buf, data, data_len);
wmi->last_mgmt_tx_frame = buf;
wmi->last_mgmt_tx_frame_len = data_len;
@@ -3682,7 +3681,7 @@ static int __ath6kl_wmi_send_mgmt_cmd(struct wmi *wmi, u8 if_idx, u32 id,
if (wait)
return -EINVAL; /* Offload for wait not supported */
- buf = kmalloc(data_len, GFP_KERNEL);
+ buf = kmemdup(data, data_len, GFP_KERNEL);
if (!buf)
return -ENOMEM;
@@ -3693,7 +3692,6 @@ static int __ath6kl_wmi_send_mgmt_cmd(struct wmi *wmi, u8 if_idx, u32 id,
}
kfree(wmi->last_mgmt_tx_frame);
- memcpy(buf, data, data_len);
wmi->last_mgmt_tx_frame = buf;
wmi->last_mgmt_tx_frame_len = data_len;
diff --git a/drivers/net/wireless/st/cw1200/queue.c b/drivers/net/wireless/st/cw1200/queue.c
index 14133eedb3b6..12952b1c29df 100644
--- a/drivers/net/wireless/st/cw1200/queue.c
+++ b/drivers/net/wireless/st/cw1200/queue.c
@@ -79,10 +79,9 @@ static void cw1200_queue_register_post_gc(struct list_head *gc_list,
struct cw1200_queue_item *item)
{
struct cw1200_queue_item *gc_item;
- gc_item = kmalloc(sizeof(struct cw1200_queue_item),
+ gc_item = kmemdup(item, sizeof(struct cw1200_queue_item),
GFP_ATOMIC);
BUG_ON(!gc_item);
- memcpy(gc_item, item, sizeof(struct cw1200_queue_item));
list_add_tail(&gc_item->head, gc_list);
}
diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c
index c9a485ecee7b..297207856494 100644
--- a/drivers/net/wireless/ti/wlcore/main.c
+++ b/drivers/net/wireless/ti/wlcore/main.c
@@ -1434,7 +1434,7 @@ int wl1271_rx_filter_alloc_field(struct wl12xx_rx_filter *filter,
field = &filter->fields[filter->num_fields];
- field->pattern = kzalloc(len, GFP_KERNEL);
+ field->pattern = kmemdup(pattern, len, GFP_KERNEL);
if (!field->pattern) {
wl1271_warning("Failed to allocate RX filter pattern");
return -ENOMEM;
@@ -1445,7 +1445,6 @@ int wl1271_rx_filter_alloc_field(struct wl12xx_rx_filter *filter,
field->offset = cpu_to_le16(offset);
field->flags = flags;
field->len = len;
- memcpy(field->pattern, pattern, len);
return 0;
}
--
2.11.0
^ permalink raw reply related
* Re: [PATCH v5 bpf-next 4/9] libbpf: add kprobe/uprobe attach API
From: Andrii Nakryiko @ 2019-07-03 16:47 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov, Kernel Team,
Yonghong Song
In-Reply-To: <5e494d84-5db9-3d57-ccb3-c619cbae7833@iogearbox.net>
On Wed, Jul 3, 2019 at 5:39 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 07/02/2019 01:58 AM, Andrii Nakryiko wrote:
> > Add ability to attach to kernel and user probes and retprobes.
> > Implementation depends on perf event support for kprobes/uprobes.
> >
> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> > Reviewed-by: Stanislav Fomichev <sdf@google.com>
> > ---
> > tools/lib/bpf/libbpf.c | 169 +++++++++++++++++++++++++++++++++++++++
> > tools/lib/bpf/libbpf.h | 7 ++
> > tools/lib/bpf/libbpf.map | 2 +
> > 3 files changed, 178 insertions(+)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index bcaa294f819d..7b6142408b15 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -4021,6 +4021,175 @@ struct bpf_link *bpf_program__attach_perf_event(struct bpf_program *prog,
> > return (struct bpf_link *)link;
> > }
> >
> > +/*
> > + * this function is expected to parse integer in the range of [0, 2^31-1] from
> > + * given file using scanf format string fmt. If actual parsed value is
> > + * negative, the result might be indistinguishable from error
> > + */
> > +static int parse_uint_from_file(const char *file, const char *fmt)
> > +{
> > + char buf[STRERR_BUFSIZE];
> > + int err, ret;
> > + FILE *f;
> > +
> > + f = fopen(file, "r");
> > + if (!f) {
> > + err = -errno;
> > + pr_debug("failed to open '%s': %s\n", file,
> > + libbpf_strerror_r(err, buf, sizeof(buf)));
> > + return err;
> > + }
> > + err = fscanf(f, fmt, &ret);
> > + if (err != 1) {
> > + err = err == EOF ? -EIO : -errno;
> > + pr_debug("failed to parse '%s': %s\n", file,
> > + libbpf_strerror_r(err, buf, sizeof(buf)));
> > + fclose(f);
> > + return err;
> > + }
> > + fclose(f);
> > + return ret;
> > +}
> > +
> > +static int determine_kprobe_perf_type(void)
> > +{
> > + const char *file = "/sys/bus/event_source/devices/kprobe/type";
> > +
> > + return parse_uint_from_file(file, "%d\n");
> > +}
> > +
> > +static int determine_uprobe_perf_type(void)
> > +{
> > + const char *file = "/sys/bus/event_source/devices/uprobe/type";
> > +
> > + return parse_uint_from_file(file, "%d\n");
> > +}
> > +
> > +static int determine_kprobe_retprobe_bit(void)
> > +{
> > + const char *file = "/sys/bus/event_source/devices/kprobe/format/retprobe";
> > +
> > + return parse_uint_from_file(file, "config:%d\n");
> > +}
> > +
> > +static int determine_uprobe_retprobe_bit(void)
> > +{
> > + const char *file = "/sys/bus/event_source/devices/uprobe/format/retprobe";
> > +
> > + return parse_uint_from_file(file, "config:%d\n");
> > +}
> > +
> > +static int perf_event_open_probe(bool uprobe, bool retprobe, const char *name,
> > + uint64_t offset, int pid)
> > +{
> > + struct perf_event_attr attr = {};
> > + char errmsg[STRERR_BUFSIZE];
> > + int type, pfd, err;
> > +
> > + type = uprobe ? determine_uprobe_perf_type()
> > + : determine_kprobe_perf_type();
> > + if (type < 0) {
> > + pr_warning("failed to determine %s perf type: %s\n",
> > + uprobe ? "uprobe" : "kprobe",
> > + libbpf_strerror_r(type, errmsg, sizeof(errmsg)));
> > + return type;
> > + }
> > + if (retprobe) {
> > + int bit = uprobe ? determine_uprobe_retprobe_bit()
> > + : determine_kprobe_retprobe_bit();
> > +
> > + if (bit < 0) {
> > + pr_warning("failed to determine %s retprobe bit: %s\n",
> > + uprobe ? "uprobe" : "kprobe",
> > + libbpf_strerror_r(bit, errmsg,
> > + sizeof(errmsg)));
> > + return bit;
> > + }
> > + attr.config |= 1 << bit;
> > + }
> > + attr.size = sizeof(attr);
> > + attr.type = type;
> > + attr.config1 = (uint64_t)(void *)name; /* kprobe_func or uprobe_path */
> > + attr.config2 = offset; /* kprobe_addr or probe_offset */
> > +
> > + /* pid filter is meaningful only for uprobes */
> > + pfd = syscall(__NR_perf_event_open, &attr,
> > + pid < 0 ? -1 : pid /* pid */,
> > + pid == -1 ? 0 : -1 /* cpu */,
> > + -1 /* group_fd */, PERF_FLAG_FD_CLOEXEC);
> > + if (pfd < 0) {
> > + err = -errno;
> > + pr_warning("%s perf_event_open() failed: %s\n",
> > + uprobe ? "uprobe" : "kprobe",
> > + libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> > + return err;
> > + }
> > + return pfd;
> > +}
> > +
> > +struct bpf_link *bpf_program__attach_kprobe(struct bpf_program *prog,
> > + bool retprobe,
> > + const char *func_name)
> > +{
> > + char errmsg[STRERR_BUFSIZE];
> > + struct bpf_link *link;
> > + int pfd, err;
> > +
> > + pfd = perf_event_open_probe(false /* uprobe */, retprobe, func_name,
> > + 0 /* offset */, -1 /* pid */);
> > + if (pfd < 0) {
> > + pr_warning("program '%s': failed to create %s '%s' perf event: %s\n",
> > + bpf_program__title(prog, false),
> > + retprobe ? "kretprobe" : "kprobe", func_name,
> > + libbpf_strerror_r(pfd, errmsg, sizeof(errmsg)));
> > + return ERR_PTR(pfd);
> > + }
> > + link = bpf_program__attach_perf_event(prog, pfd);
> > + if (IS_ERR(link)) {
> > + close(pfd);
> > + err = PTR_ERR(link);
> > + pr_warning("program '%s': failed to attach to %s '%s': %s\n",
> > + bpf_program__title(prog, false),
> > + retprobe ? "kretprobe" : "kprobe", func_name,
> > + libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> > + return link;
> > + }
> > + return link;
> > +}
> > +
> > +struct bpf_link *bpf_program__attach_uprobe(struct bpf_program *prog,
> > + bool retprobe, pid_t pid,
> > + const char *binary_path,
> > + size_t func_offset)
> > +{
> > + char errmsg[STRERR_BUFSIZE];
> > + struct bpf_link *link;
> > + int pfd, err;
> > +
> > + pfd = perf_event_open_probe(true /* uprobe */, retprobe,
> > + binary_path, func_offset, pid);
> > + if (pfd < 0) {
> > + pr_warning("program '%s': failed to create %s '%s:0x%zx' perf event: %s\n",
> > + bpf_program__title(prog, false),
> > + retprobe ? "uretprobe" : "uprobe",
> > + binary_path, func_offset,
> > + libbpf_strerror_r(pfd, errmsg, sizeof(errmsg)));
> > + return ERR_PTR(pfd);
> > + }
> > + link = bpf_program__attach_perf_event(prog, pfd);
> > + if (IS_ERR(link)) {
> > + close(pfd);
> > + err = PTR_ERR(link);
> > + pr_warning("program '%s': failed to attach to %s '%s:0x%zx': %s\n",
> > + bpf_program__title(prog, false),
> > + retprobe ? "uretprobe" : "uprobe",
> > + binary_path, func_offset,
> > + libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> > + return link;
> > + }
> > + return link;
> > +}
>
> Hm, this only addresses half the feedback I had in prior version [0]. Patch 2/9
Hi Daniel,
Yes, and I explained why in reply to your original email, please see [1].
I've started with exactly separation you wanted, but it turned out to
be cumbersome and harder to use user API, while also somewhat more
complicated to implement. Mostly because in that design bpf_link
exists in two states: created-but-not-attached and attached. Which
forces user to do additional clean ups if creation succeeded, but
attachment failed. It also makes it a bit harder to provide good
contextual error logging if something goes wrong, because not all
original parameters are preserved, as some of them might be needed
only for creation, but not attachment (or we'll have to allocate and
copy extra stuff just for logging purposes).
On the other hand, having separate generic attach_event method doesn't
help that much, as there is little common functionality to reuse
across all kinds of possible bpf_link types.
[1] https://lore.kernel.org/bpf/20190621045555.4152743-4-andriin@fb.com/T/#m6cfc141e7b57970bc948134bf671a46972b95134
> with bpf_link with destructor looks good to me, but my feedback from back then was
> that all the kprobe/uprobe/tracepoint/raw_tracepoint should be split API-wise, so
> you'll end up with something like the below, that is, 1) a set of functions that
> only /create/ the bpf_link handle /once/, and 2) a helper that allows /attaching/
> progs to one or multiple bpf_links. The set of APIs would look like:
>
> struct bpf_link *bpf_link__create_kprobe(bool retprobe, const char *func_name);
> struct bpf_link *bpf_link__create_uprobe(bool retprobe, pid_t pid,
> const char *binary_path,
> size_t func_offset);
> int bpf_program__attach_to_link(struct bpf_link *link, struct bpf_program *prog);
> int bpf_link__destroy(struct bpf_link *link);
>
> This seems much more natural to me. Right now you sort of do both in one single API.
It felt that way for me as well, until I implemented it and used it in
selftests. And then it felt unnecessarily verbose without giving any
benefit. I still have a local patchset with that change, I can post it
as RFC, if you don't trust my judgement. Please let me know.
> Detangling the bpf_program__attach_{uprobe,kprobe}() would also avoid that you have
> to redo all the perf_event_open_probe() work over and over in order to get the pfd
What do you mean by "redo all the perf_event_open_probe work"? In
terms of code, I just reuse the same function, so there is no
duplicate code. And in either design you'll have to open that
perf_event, so that work will have to be done one way or another.
> context where you can later attach something to. Given bpf_program__attach_to_link()
> API, you also wouldn't need to expose the bpf_program__attach_perf_event() from
I'd expose attach_perf_event either way, it's high-level API I want to
provide, we have use cases where user is creating some specific
non-kprobe/non-tracepoint perf events and wants to attach to it. E.g.,
HW counter overflow events for CPU profilers. So that API is not some
kind of leaked abstraction, it's something I want to have anyway.
> patch 3/9. Thoughts?
I believe this hybrid approach provides better usability without
compromising anything. The only theoretical benefit of complete
separation of bpf_link creation and attachment is that user code would
be able to separate those two steps code organization-wise. But it's
easily doable through custom application code (just encapsulate all
the parameters and type of attachment and pass it around until you
actually need to attach), but I don't think it's necessary in practice
(so far I never needed anything like that).
Hope I convinced you that while elegant, it's not that practical. Also
hybrid approach isn't inelegant either and doesn't produce code
duplication (it actually eliminates some unnecessary allocations,
e.g., for storing tp_name for raw_tracepoint attach) :)
>
> [0] https://lore.kernel.org/bpf/a7780057-1d70-9ace-960b-ff65867dc277@iogearbox.net/
>
> > enum bpf_perf_event_ret
> > bpf_perf_event_read_simple(void *mmap_mem, size_t mmap_size, size_t page_size,
> > void **copy_mem, size_t *copy_size,
> > diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> > index 1bf66c4a9330..bd767cc11967 100644
> > --- a/tools/lib/bpf/libbpf.h
> > +++ b/tools/lib/bpf/libbpf.h
> > @@ -171,6 +171,13 @@ LIBBPF_API int bpf_link__destroy(struct bpf_link *link);
> >
> > LIBBPF_API struct bpf_link *
> > bpf_program__attach_perf_event(struct bpf_program *prog, int pfd);
> > +LIBBPF_API struct bpf_link *
> > +bpf_program__attach_kprobe(struct bpf_program *prog, bool retprobe,
> > + const char *func_name);
> > +LIBBPF_API struct bpf_link *
> > +bpf_program__attach_uprobe(struct bpf_program *prog, bool retprobe,
> > + pid_t pid, const char *binary_path,
> > + size_t func_offset);
> >
> > struct bpf_insn;
> >
> > diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> > index 756f5aa802e9..57a40fb60718 100644
> > --- a/tools/lib/bpf/libbpf.map
> > +++ b/tools/lib/bpf/libbpf.map
> > @@ -169,7 +169,9 @@ LIBBPF_0.0.4 {
> > global:
> > bpf_link__destroy;
> > bpf_object__load_xattr;
> > + bpf_program__attach_kprobe;
> > bpf_program__attach_perf_event;
> > + bpf_program__attach_uprobe;
> > btf_dump__dump_type;
> > btf_dump__free;
> > btf_dump__new;
> >
>
^ permalink raw reply
* Re: [PATCH bpf v2] xdp: fix race on generic receive path
From: William Tu @ 2019-07-03 16:48 UTC (permalink / raw)
To: Magnus Karlsson
Cc: Ilya Maximets, Network Development, LKML, bpf, Xdp,
David S. Miller, Björn Töpel, Magnus Karlsson,
Jonathan Lemon, Jakub Kicinski, Alexei Starovoitov,
Daniel Borkmann
In-Reply-To: <CAJ8uoz1Wr+bJrO+HNtSD5b79ych-pNg7BxFiHVhzaMSGGAdqLA@mail.gmail.com>
On Wed, Jul 3, 2019 at 6:20 AM Magnus Karlsson
<magnus.karlsson@gmail.com> wrote:
>
> On Wed, Jul 3, 2019 at 2:09 PM Ilya Maximets <i.maximets@samsung.com> wrote:
> >
> > Unlike driver mode, generic xdp receive could be triggered
> > by different threads on different CPU cores at the same time
> > leading to the fill and rx queue breakage. For example, this
> > could happen while sending packets from two processes to the
> > first interface of veth pair while the second part of it is
> > open with AF_XDP socket.
> >
> > Need to take a lock for each generic receive to avoid race.
>
> I measured the performance degradation of rxdrop on my local machine
> and it went from 2.19 to 2.08, so roughly a 5% drop. I think we can
> live with this in XDP_SKB mode. If we at some later point in time need
> to boost performance in this mode, let us look at it then from a
> broader perspective and find the most low hanging fruit.
>
> Thanks Ilya for this fix.
>
> Acked-by: Magnus Karlsson <magnus.karlsson@intel.com>
>
> > Fixes: c497176cb2e4 ("xsk: add Rx receive functions and poll support")
> > Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
> > ---
Tested on my machine and works ok.
Tested-by: William Tu <u9012063@gmail.com>
^ permalink raw reply
* [PATCH][next] gve: fix -ENOMEM null check on a page allocation
From: Colin King @ 2019-07-03 16:50 UTC (permalink / raw)
To: Catherine Sullivan, Sagi Shahar, Jon Olson, David S . Miller,
Willem de Bruijn, netdev
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
Currently the check to see if a page is allocated is incorrect
and is checking if the pointer page is null, not *page as
intended. Fix this.
Addresses-Coverity: ("Dereference before null check")
Fixes: f5cedc84a30d ("gve: Add transmit and receive support")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/net/ethernet/google/gve/gve_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/google/gve/gve_main.c b/drivers/net/ethernet/google/gve/gve_main.c
index 6a147ed4627f..6ea74c364a4b 100644
--- a/drivers/net/ethernet/google/gve/gve_main.c
+++ b/drivers/net/ethernet/google/gve/gve_main.c
@@ -516,7 +516,7 @@ int gve_alloc_page(struct device *dev, struct page **page, dma_addr_t *dma,
enum dma_data_direction dir)
{
*page = alloc_page(GFP_KERNEL);
- if (!page)
+ if (!*page)
return -ENOMEM;
*dma = dma_map_page(dev, *page, 0, PAGE_SIZE, dir);
if (dma_mapping_error(dev, *dma)) {
--
2.20.1
^ permalink raw reply related
* [PATCH bpf-next RFC v3 0/6] bpf: add BPF_MAP_DUMP command to dump more than one entry per call
From: Brian Vazquez @ 2019-07-03 17:01 UTC (permalink / raw)
To: Brian Vazquez, Alexei Starovoitov, Daniel Borkmann,
David S . Miller
Cc: Stanislav Fomichev, Willem de Bruijn, Petar Penkov, linux-kernel,
netdev, bpf, Brian Vazquez
This introduces a new command to retrieve a variable number of entries
from a bpf map.
This new command can be executed from the existing BPF syscall as
follows:
err = bpf(BPF_MAP_DUMP, union bpf_attr *attr, u32 size)
using attr->dump.map_fd, attr->dump.prev_key, attr->dump.buf,
attr->dump.buf_len
returns zero or negative error, and populates buf and buf_len on
succees
This implementation is wrapping the existing bpf methods:
map_get_next_key and map_lookup_elem
the results show that even with a 1-elem_size buffer, it runs ~40 faster
than the current implementation, improvements of ~85% are reported when
the buffer size is increased, although, after the buffer size is around
5% of the total number of entries there's no huge difference in
increasing it.
Tested:
Tried different size buffers to handle case where the bulk is bigger, or
the elements to retrieve are less than the existing ones, all runs read
a map of 100K entries. Below are the results(in ns) from the different
runs:
buf_len_1: 69038725 entry-by-entry: 112384424 improvement
38.569134
buf_len_2: 40897447 entry-by-entry: 111030546 improvement
63.165590
buf_len_230: 13652714 entry-by-entry: 111694058 improvement
87.776687
buf_len_5000: 13576271 entry-by-entry: 111101169 improvement
87.780263
buf_len_73000: 14694343 entry-by-entry: 111740162 improvement
86.849542
buf_len_100000: 13745969 entry-by-entry: 114151991 improvement
87.958187
buf_len_234567: 14329834 entry-by-entry: 114427589 improvement
87.476941
Changelog:
v3:
- add explanation of the API in the commit message
- fix masked errors and return them to user
- copy last_key from return buf into prev_key if it was provided
- run test with kpti and retpoline mitigations
v2:
- use proper bpf-next tag
Brian Vazquez (6):
bpf: add bpf_map_value_size and bp_map_copy_value helper functions
bpf: add BPF_MAP_DUMP command to dump more than one entry per call
bpf: keep bpf.h in sync with tools/
libbpf: support BPF_MAP_DUMP command
selftests/bpf: test BPF_MAP_DUMP command on a bpf hashmap
selftests/bpf: add test to measure performance of BPF_MAP_DUMP
include/uapi/linux/bpf.h | 9 +
kernel/bpf/syscall.c | 252 ++++++++++++++++++------
tools/include/uapi/linux/bpf.h | 9 +
tools/lib/bpf/bpf.c | 28 +++
tools/lib/bpf/bpf.h | 4 +
tools/lib/bpf/libbpf.map | 2 +
tools/testing/selftests/bpf/test_maps.c | 147 +++++++++++++-
7 files changed, 388 insertions(+), 63 deletions(-)
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply
* [PATCH bpf-next RFC v3 4/6] libbpf: support BPF_MAP_DUMP command
From: Brian Vazquez @ 2019-07-03 17:01 UTC (permalink / raw)
To: Brian Vazquez, Alexei Starovoitov, Daniel Borkmann,
David S . Miller
Cc: Stanislav Fomichev, Willem de Bruijn, Petar Penkov, linux-kernel,
netdev, bpf, Brian Vazquez
In-Reply-To: <20190703170118.196552-1-brianvv@google.com>
Make libbpf aware of new BPF_MAP_DUMP command and add bpf_map_dump and
bpf_map_dump_flags to use them from the library.
Suggested-by: Stanislav Fomichev <sdf@google.com>
Signed-off-by: Brian Vazquez <brianvv@google.com>
---
tools/lib/bpf/bpf.c | 28 ++++++++++++++++++++++++++++
tools/lib/bpf/bpf.h | 4 ++++
tools/lib/bpf/libbpf.map | 2 ++
3 files changed, 34 insertions(+)
diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
index c7d7993c44bb..c1139b7db756 100644
--- a/tools/lib/bpf/bpf.c
+++ b/tools/lib/bpf/bpf.c
@@ -368,6 +368,34 @@ int bpf_map_update_elem(int fd, const void *key, const void *value,
return sys_bpf(BPF_MAP_UPDATE_ELEM, &attr, sizeof(attr));
}
+int bpf_map_dump(int fd, const void *prev_key, void *buf, void *buf_len)
+{
+ union bpf_attr attr;
+
+ memset(&attr, 0, sizeof(attr));
+ attr.dump.map_fd = fd;
+ attr.dump.prev_key = ptr_to_u64(prev_key);
+ attr.dump.buf = ptr_to_u64(buf);
+ attr.dump.buf_len = ptr_to_u64(buf_len);
+
+ return sys_bpf(BPF_MAP_DUMP, &attr, sizeof(attr));
+}
+
+int bpf_map_dump_flags(int fd, const void *prev_key, void *buf, void *buf_len,
+ __u64 flags)
+{
+ union bpf_attr attr;
+
+ memset(&attr, 0, sizeof(attr));
+ attr.dump.map_fd = fd;
+ attr.dump.prev_key = ptr_to_u64(prev_key);
+ attr.dump.buf = ptr_to_u64(buf);
+ attr.dump.buf_len = ptr_to_u64(buf_len);
+ attr.dump.flags = flags;
+
+ return sys_bpf(BPF_MAP_DUMP, &attr, sizeof(attr));
+}
+
int bpf_map_lookup_elem(int fd, const void *key, void *value)
{
union bpf_attr attr;
diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
index ff42ca043dc8..86496443440e 100644
--- a/tools/lib/bpf/bpf.h
+++ b/tools/lib/bpf/bpf.h
@@ -112,6 +112,10 @@ LIBBPF_API int bpf_verify_program(enum bpf_prog_type type,
LIBBPF_API int bpf_map_update_elem(int fd, const void *key, const void *value,
__u64 flags);
+LIBBPF_API int bpf_map_dump(int fd, const void *prev_key, void *buf,
+ void *buf_len);
+LIBBPF_API int bpf_map_dump_flags(int fd, const void *prev_key, void *buf,
+ void *buf_len, __u64 flags);
LIBBPF_API int bpf_map_lookup_elem(int fd, const void *key, void *value);
LIBBPF_API int bpf_map_lookup_elem_flags(int fd, const void *key, void *value,
__u64 flags);
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index 2c6d835620d2..e7641773cfb0 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -173,4 +173,6 @@ LIBBPF_0.0.4 {
btf__parse_elf;
bpf_object__load_xattr;
libbpf_num_possible_cpus;
+ bpf_map_dump;
+ bpf_map_dump_flags;
} LIBBPF_0.0.3;
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH bpf-next RFC v3 6/6] selftests/bpf: add test to measure performance of BPF_MAP_DUMP
From: Brian Vazquez @ 2019-07-03 17:01 UTC (permalink / raw)
To: Brian Vazquez, Alexei Starovoitov, Daniel Borkmann,
David S . Miller
Cc: Stanislav Fomichev, Willem de Bruijn, Petar Penkov, linux-kernel,
netdev, bpf, Brian Vazquez
In-Reply-To: <20190703170118.196552-1-brianvv@google.com>
This tests compares the amount of time that takes to read an entire
table of 100K elements on a bpf hashmap using both BPF_MAP_DUMP and
BPF_MAP_GET_NEXT_KEY + BPF_MAP_LOOKUP_ELEM.
Signed-off-by: Brian Vazquez <brianvv@google.com>
---
tools/testing/selftests/bpf/test_maps.c | 65 +++++++++++++++++++++++++
1 file changed, 65 insertions(+)
diff --git a/tools/testing/selftests/bpf/test_maps.c b/tools/testing/selftests/bpf/test_maps.c
index b19ba6aa8e36..786d0e340aed 100644
--- a/tools/testing/selftests/bpf/test_maps.c
+++ b/tools/testing/selftests/bpf/test_maps.c
@@ -18,6 +18,7 @@
#include <sys/socket.h>
#include <netinet/in.h>
#include <linux/bpf.h>
+#include <linux/time64.h>
#include <bpf/bpf.h>
#include <bpf/libbpf.h>
@@ -388,6 +389,69 @@ static void test_hashmap_dump(void)
close(fd);
}
+static void test_hashmap_dump_perf(void)
+{
+ int fd, i, max_entries = 100000;
+ uint64_t key, value, next_key;
+ bool next_key_valid = true;
+ void *buf;
+ u32 buf_len, entries;
+ int j = 0;
+ int clk_id = CLOCK_MONOTONIC;
+ struct timespec begin, end;
+ long long time_spent, dump_time_spent;
+ double res;
+ int tests[] = {1, 2, 230, 5000, 73000, 100000, 234567};
+ int test_len = ARRAY_SIZE(tests);
+ const int elem_size = sizeof(key) + sizeof(value);
+
+ fd = helper_fill_hashmap(max_entries);
+ // Alloc memory considering the largest buffer
+ buf = malloc(elem_size * tests[test_len-1]);
+ assert(buf != NULL);
+
+test:
+ entries = tests[j];
+ buf_len = elem_size*tests[j];
+ j++;
+ clock_gettime(clk_id, &begin);
+ errno = 0;
+ i = 0;
+ while (errno == 0) {
+ bpf_map_dump(fd, !i ? NULL : &key,
+ buf, &buf_len);
+ if (errno)
+ break;
+ if (!i)
+ key = *((uint64_t *)(buf + buf_len - elem_size));
+ i += buf_len / elem_size;
+ }
+ clock_gettime(clk_id, &end);
+ assert(i == max_entries);
+ dump_time_spent = NSEC_PER_SEC * (end.tv_sec - begin.tv_sec) +
+ end.tv_nsec - begin.tv_nsec;
+ next_key_valid = true;
+ clock_gettime(clk_id, &begin);
+ assert(bpf_map_get_next_key(fd, NULL, &key) == 0);
+ for (i = 0; next_key_valid; i++) {
+ next_key_valid = bpf_map_get_next_key(fd, &key, &next_key) == 0;
+ assert(bpf_map_lookup_elem(fd, &key, &value) == 0);
+ key = next_key;
+ }
+ clock_gettime(clk_id, &end);
+ time_spent = NSEC_PER_SEC * (end.tv_sec - begin.tv_sec) +
+ end.tv_nsec - begin.tv_nsec;
+ res = (1-((double)dump_time_spent/time_spent))*100;
+ printf("buf_len_%u:\t %llu entry-by-entry: %llu improvement %lf\n",
+ entries, dump_time_spent, time_spent, res);
+ assert(i == max_entries);
+
+ if (j < test_len)
+ goto test;
+ free(buf);
+ close(fd);
+}
+
static void test_hashmap_zero_seed(void)
{
int i, first, second, old_flags;
@@ -1748,6 +1812,7 @@ static void run_all_tests(void)
test_hashmap_walk(0, NULL);
test_hashmap_zero_seed();
test_hashmap_dump();
+ test_hashmap_dump_perf();
test_arraymap(0, NULL);
test_arraymap_percpu(0, NULL);
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH bpf-next RFC v3 5/6] selftests/bpf: test BPF_MAP_DUMP command on a bpf hashmap
From: Brian Vazquez @ 2019-07-03 17:01 UTC (permalink / raw)
To: Brian Vazquez, Alexei Starovoitov, Daniel Borkmann,
David S . Miller
Cc: Stanislav Fomichev, Willem de Bruijn, Petar Penkov, linux-kernel,
netdev, bpf, Brian Vazquez
In-Reply-To: <20190703170118.196552-1-brianvv@google.com>
This tests exercise the new command on a bpf hashmap and make sure it
works as expected.
Signed-off-by: Brian Vazquez <brianvv@google.com>
---
tools/testing/selftests/bpf/test_maps.c | 82 ++++++++++++++++++++++++-
1 file changed, 80 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_maps.c b/tools/testing/selftests/bpf/test_maps.c
index a3fbc571280a..b19ba6aa8e36 100644
--- a/tools/testing/selftests/bpf/test_maps.c
+++ b/tools/testing/selftests/bpf/test_maps.c
@@ -309,6 +309,85 @@ static void test_hashmap_walk(unsigned int task, void *data)
close(fd);
}
+static void test_hashmap_dump(void)
+{
+ int fd, i, max_entries = 5;
+ uint64_t keys[max_entries], values[max_entries];
+ uint64_t key, value, next_key, prev_key;
+ bool next_key_valid = true;
+ void *buf, *elem;
+ u32 buf_len;
+ const int elem_size = sizeof(key) + sizeof(value);
+
+ fd = helper_fill_hashmap(max_entries);
+
+ // Get the elements in the hashmap, and store them in that order
+ assert(bpf_map_get_next_key(fd, NULL, &key) == 0);
+ i = 0;
+ keys[i] = key;
+ for (i = 1; next_key_valid; i++) {
+ next_key_valid = bpf_map_get_next_key(fd, &key, &next_key) == 0;
+ assert(bpf_map_lookup_elem(fd, &key, &values[i - 1]) == 0);
+ keys[i-1] = key;
+ key = next_key;
+ }
+
+ // Alloc memory for the whole table
+ buf = malloc(elem_size * max_entries);
+ assert(buf != NULL);
+
+ // Check that buf_len < elem_size returns EINVAL
+ buf_len = elem_size-1;
+ errno = 0;
+ assert(bpf_map_dump(fd, NULL, buf, &buf_len) == -1 && errno == EINVAL);
+
+ // Check that it returns the first two elements
+ errno = 0;
+ buf_len = elem_size * 2;
+ i = 0;
+ assert(bpf_map_dump(fd, NULL, buf, &buf_len) == 0 &&
+ buf_len == 2*elem_size);
+ elem = buf;
+ assert((*(uint64_t *)elem) == keys[i] &&
+ (*(uint64_t *)(elem + sizeof(key))) == values[i]);
+ elem = buf + elem_size;
+ i++;
+ assert((*(uint64_t *)elem) == keys[i] &&
+ (*(uint64_t *)(elem + sizeof(key))) == values[i]);
+ i++;
+
+ /* Check that prev_key contains key from last_elem retrieved in previous
+ * call
+ */
+ prev_key = *((uint64_t *)elem);
+ assert(bpf_map_dump(fd, &prev_key, buf, &buf_len) == 0 &&
+ buf_len == elem_size*2);
+ elem = buf;
+ assert((*(uint64_t *)elem) == keys[i] &&
+ (*(uint64_t *)(elem + sizeof(key))) == values[i]);
+ elem = buf + elem_size;
+ i++;
+ assert((*(uint64_t *)elem) == keys[i] &&
+ (*(uint64_t *)(elem + sizeof(key))) == values[i]);
+ i++;
+ assert(prev_key == (*(uint64_t *)elem));
+
+ /* Continue reading from map and verify buf_len only contains 1 element
+ * even though buf_len is 2 elem_size.
+ */
+ assert(bpf_map_dump(fd, &prev_key, buf, &buf_len) == 0 &&
+ buf_len == elem_size);
+ elem = buf;
+ assert((*(uint64_t *)elem) == keys[i] &&
+ (*(uint64_t *)(elem + sizeof(key))) == values[i]);
+
+ assert(bpf_map_dump(fd, &prev_key, buf, &buf_len) == -1 &&
+ errno == ENOENT);
+
+ free(buf);
+ close(fd);
+}
+
static void test_hashmap_zero_seed(void)
{
int i, first, second, old_flags;
@@ -1668,6 +1747,7 @@ static void run_all_tests(void)
test_hashmap_percpu(0, NULL);
test_hashmap_walk(0, NULL);
test_hashmap_zero_seed();
+ test_hashmap_dump();
test_arraymap(0, NULL);
test_arraymap_percpu(0, NULL);
@@ -1705,11 +1785,9 @@ int main(void)
map_flags = BPF_F_NO_PREALLOC;
run_all_tests();
-
#define CALL
#include <map_tests/tests.h>
#undef CALL
-
printf("test_maps: OK, %d SKIPPED\n", skips);
return 0;
}
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH bpf-next RFC v3 3/6] bpf: keep bpf.h in sync with tools/
From: Brian Vazquez @ 2019-07-03 17:01 UTC (permalink / raw)
To: Brian Vazquez, Alexei Starovoitov, Daniel Borkmann,
David S . Miller
Cc: Stanislav Fomichev, Willem de Bruijn, Petar Penkov, linux-kernel,
netdev, bpf, Brian Vazquez
In-Reply-To: <20190703170118.196552-1-brianvv@google.com>
Adds bpf_attr.dump structure to libbpf.
Suggested-by: Stanislav Fomichev <sdf@google.com>
Signed-off-by: Brian Vazquez <brianvv@google.com>
---
tools/include/uapi/linux/bpf.h | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index a396b516a2b2..db6785a03538 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -106,6 +106,7 @@ enum bpf_cmd {
BPF_TASK_FD_QUERY,
BPF_MAP_LOOKUP_AND_DELETE_ELEM,
BPF_MAP_FREEZE,
+ BPF_MAP_DUMP,
};
enum bpf_map_type {
@@ -388,6 +389,14 @@ union bpf_attr {
__u64 flags;
};
+ struct { /* struct used by BPF_MAP_DUMP command */
+ __u32 map_fd;
+ __aligned_u64 prev_key;
+ __aligned_u64 buf;
+ __aligned_u64 buf_len; /* input/output: len of buf */
+ __u64 flags;
+ } dump;
+
struct { /* anonymous struct used by BPF_PROG_LOAD command */
__u32 prog_type; /* one of enum bpf_prog_type */
__u32 insn_cnt;
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH bpf-next RFC v3 1/6] bpf: add bpf_map_value_size and bp_map_copy_value helper functions
From: Brian Vazquez @ 2019-07-03 17:01 UTC (permalink / raw)
To: Brian Vazquez, Alexei Starovoitov, Daniel Borkmann,
David S . Miller
Cc: Stanislav Fomichev, Willem de Bruijn, Petar Penkov, linux-kernel,
netdev, bpf, Brian Vazquez
In-Reply-To: <20190703170118.196552-1-brianvv@google.com>
Move reusable code from map_lookup_elem to helper functions to avoid code
duplication in kernel/bpf/syscall.c
Suggested-by: Stanislav Fomichev <sdf@google.com>
Signed-off-by: Brian Vazquez <brianvv@google.com>
---
kernel/bpf/syscall.c | 134 +++++++++++++++++++++++--------------------
1 file changed, 73 insertions(+), 61 deletions(-)
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index b0f545e07425..d200d2837ade 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -126,6 +126,76 @@ static struct bpf_map *find_and_alloc_map(union bpf_attr *attr)
return map;
}
+static u32 bpf_map_value_size(struct bpf_map *map)
+{
+ if (map->map_type == BPF_MAP_TYPE_PERCPU_HASH ||
+ map->map_type == BPF_MAP_TYPE_LRU_PERCPU_HASH ||
+ map->map_type == BPF_MAP_TYPE_PERCPU_ARRAY ||
+ map->map_type == BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE)
+ return round_up(map->value_size, 8) * num_possible_cpus();
+ else if (IS_FD_MAP(map))
+ return sizeof(u32);
+ else
+ return map->value_size;
+}
+
+static int bpf_map_copy_value(struct bpf_map *map, void *key, void *value,
+ __u64 flags)
+{
+ void *ptr;
+ int err;
+
+ if (bpf_map_is_dev_bound(map))
+ return bpf_map_offload_lookup_elem(map, key, value);
+
+ preempt_disable();
+ this_cpu_inc(bpf_prog_active);
+ if (map->map_type == BPF_MAP_TYPE_PERCPU_HASH ||
+ map->map_type == BPF_MAP_TYPE_LRU_PERCPU_HASH) {
+ err = bpf_percpu_hash_copy(map, key, value);
+ } else if (map->map_type == BPF_MAP_TYPE_PERCPU_ARRAY) {
+ err = bpf_percpu_array_copy(map, key, value);
+ } else if (map->map_type == BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE) {
+ err = bpf_percpu_cgroup_storage_copy(map, key, value);
+ } else if (map->map_type == BPF_MAP_TYPE_STACK_TRACE) {
+ err = bpf_stackmap_copy(map, key, value);
+ } else if (IS_FD_ARRAY(map)) {
+ err = bpf_fd_array_map_lookup_elem(map, key, value);
+ } else if (IS_FD_HASH(map)) {
+ err = bpf_fd_htab_map_lookup_elem(map, key, value);
+ } else if (map->map_type == BPF_MAP_TYPE_REUSEPORT_SOCKARRAY) {
+ err = bpf_fd_reuseport_array_lookup_elem(map, key, value);
+ } else if (map->map_type == BPF_MAP_TYPE_QUEUE ||
+ map->map_type == BPF_MAP_TYPE_STACK) {
+ err = map->ops->map_peek_elem(map, value);
+ } else {
+ rcu_read_lock();
+ if (map->ops->map_lookup_elem_sys_only)
+ ptr = map->ops->map_lookup_elem_sys_only(map, key);
+ else
+ ptr = map->ops->map_lookup_elem(map, key);
+ if (IS_ERR(ptr)) {
+ err = PTR_ERR(ptr);
+ } else if (!ptr) {
+ err = -ENOENT;
+ } else {
+ err = 0;
+ if (flags & BPF_F_LOCK)
+ /* lock 'ptr' and copy everything but lock */
+ copy_map_value_locked(map, value, ptr, true);
+ else
+ copy_map_value(map, value, ptr);
+ /* mask lock, since value wasn't zero inited */
+ check_and_init_map_lock(map, value);
+ }
+ rcu_read_unlock();
+ }
+ this_cpu_dec(bpf_prog_active);
+ preempt_enable();
+
+ return err;
+}
+
void *bpf_map_area_alloc(size_t size, int numa_node)
{
/* We really just want to fail instead of triggering OOM killer
@@ -729,7 +799,7 @@ static int map_lookup_elem(union bpf_attr *attr)
void __user *uvalue = u64_to_user_ptr(attr->value);
int ufd = attr->map_fd;
struct bpf_map *map;
- void *key, *value, *ptr;
+ void *key, *value;
u32 value_size;
struct fd f;
int err;
@@ -761,72 +831,14 @@ static int map_lookup_elem(union bpf_attr *attr)
goto err_put;
}
- if (map->map_type == BPF_MAP_TYPE_PERCPU_HASH ||
- map->map_type == BPF_MAP_TYPE_LRU_PERCPU_HASH ||
- map->map_type == BPF_MAP_TYPE_PERCPU_ARRAY ||
- map->map_type == BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE)
- value_size = round_up(map->value_size, 8) * num_possible_cpus();
- else if (IS_FD_MAP(map))
- value_size = sizeof(u32);
- else
- value_size = map->value_size;
+ value_size = bpf_map_value_size(map);
err = -ENOMEM;
value = kmalloc(value_size, GFP_USER | __GFP_NOWARN);
if (!value)
goto free_key;
- if (bpf_map_is_dev_bound(map)) {
- err = bpf_map_offload_lookup_elem(map, key, value);
- goto done;
- }
-
- preempt_disable();
- this_cpu_inc(bpf_prog_active);
- if (map->map_type == BPF_MAP_TYPE_PERCPU_HASH ||
- map->map_type == BPF_MAP_TYPE_LRU_PERCPU_HASH) {
- err = bpf_percpu_hash_copy(map, key, value);
- } else if (map->map_type == BPF_MAP_TYPE_PERCPU_ARRAY) {
- err = bpf_percpu_array_copy(map, key, value);
- } else if (map->map_type == BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE) {
- err = bpf_percpu_cgroup_storage_copy(map, key, value);
- } else if (map->map_type == BPF_MAP_TYPE_STACK_TRACE) {
- err = bpf_stackmap_copy(map, key, value);
- } else if (IS_FD_ARRAY(map)) {
- err = bpf_fd_array_map_lookup_elem(map, key, value);
- } else if (IS_FD_HASH(map)) {
- err = bpf_fd_htab_map_lookup_elem(map, key, value);
- } else if (map->map_type == BPF_MAP_TYPE_REUSEPORT_SOCKARRAY) {
- err = bpf_fd_reuseport_array_lookup_elem(map, key, value);
- } else if (map->map_type == BPF_MAP_TYPE_QUEUE ||
- map->map_type == BPF_MAP_TYPE_STACK) {
- err = map->ops->map_peek_elem(map, value);
- } else {
- rcu_read_lock();
- if (map->ops->map_lookup_elem_sys_only)
- ptr = map->ops->map_lookup_elem_sys_only(map, key);
- else
- ptr = map->ops->map_lookup_elem(map, key);
- if (IS_ERR(ptr)) {
- err = PTR_ERR(ptr);
- } else if (!ptr) {
- err = -ENOENT;
- } else {
- err = 0;
- if (attr->flags & BPF_F_LOCK)
- /* lock 'ptr' and copy everything but lock */
- copy_map_value_locked(map, value, ptr, true);
- else
- copy_map_value(map, value, ptr);
- /* mask lock, since value wasn't zero inited */
- check_and_init_map_lock(map, value);
- }
- rcu_read_unlock();
- }
- this_cpu_dec(bpf_prog_active);
- preempt_enable();
-
-done:
+ err = bpf_map_copy_value(map, key, value, attr->flags);
if (err)
goto free_value;
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH bpf-next RFC v3 2/6] bpf: add BPF_MAP_DUMP command to dump more than one entry per call
From: Brian Vazquez @ 2019-07-03 17:01 UTC (permalink / raw)
To: Brian Vazquez, Alexei Starovoitov, Daniel Borkmann,
David S . Miller
Cc: Stanislav Fomichev, Willem de Bruijn, Petar Penkov, linux-kernel,
netdev, bpf, Brian Vazquez
In-Reply-To: <20190703170118.196552-1-brianvv@google.com>
This introduces a new command to retrieve a variable number of entries
from a bpf map wrapping the existing bpf methods:
map_get_next_key and map_lookup_elem
To start dumping the map from the beginning you must specify NULL as
the prev_key.
The new API returns 0 when it successfully copied all the elements
requested or it copied less because there weren't more elements to
retrieved (err == -ENOENT). In last scenario err will be masked to 0.
On a successful call buf and buf_len will contain correct data and in
case prev_key was provided (not for the first walk, since prev_key is
NULL) it will contain the last_key copied into the buf which will simplify
next call.
Only when it can't find a single element it will return -ENOENT meaning
that the map has been entirely walked. When an error is return buf,
buf_len and prev_key shouldn't be read nor used.
Because maps can be called from userspace and kernel code, this function
can have a scenario where the next_key was found but by the time we
try to retrieve the value the element is not there, in this case the
function continues and tries to get a new next_key value, skipping the
deleted key. If at some point the function find itself trap in a loop,
it will return -EINTR.
The function will try to fit as much as possible in the buf provided and
will return -EINVAL if buf_len is smaller than elem_size.
QUEUE and STACK maps are not supported.
Note that map_dump doesn't guarantee that reading the entire table is
consistent since this function is always racing with kernel and user code
but the same behaviour is found when the entire table is walked using
the current interfaces: map_get_next_key + map_lookup_elem.
It is also important to note that when a locked map the lock is grabbed for
1 entry at the time, meaning that the buf returned might or might not be
consistent.
Suggested-by: Stanislav Fomichev <sdf@google.com>
Signed-off-by: Brian Vazquez <brianvv@google.com>
---
include/uapi/linux/bpf.h | 9 +++
kernel/bpf/syscall.c | 118 +++++++++++++++++++++++++++++++++++++++
2 files changed, 127 insertions(+)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index cffea1826a1f..cc589570a639 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -106,6 +106,7 @@ enum bpf_cmd {
BPF_TASK_FD_QUERY,
BPF_MAP_LOOKUP_AND_DELETE_ELEM,
BPF_MAP_FREEZE,
+ BPF_MAP_DUMP,
};
enum bpf_map_type {
@@ -388,6 +389,14 @@ union bpf_attr {
__u64 flags;
};
+ struct { /* struct used by BPF_MAP_DUMP command */
+ __u32 map_fd;
+ __aligned_u64 prev_key;
+ __aligned_u64 buf;
+ __aligned_u64 buf_len; /* input/output: len of buf */
+ __u64 flags;
+ } dump;
+
struct { /* anonymous struct used by BPF_PROG_LOAD command */
__u32 prog_type; /* one of enum bpf_prog_type */
__u32 insn_cnt;
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index d200d2837ade..78d55463fc76 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1097,6 +1097,121 @@ static int map_get_next_key(union bpf_attr *attr)
return err;
}
+/* last field in 'union bpf_attr' used by this command */
+#define BPF_MAP_DUMP_LAST_FIELD dump.buf_len
+
+static int map_dump(union bpf_attr *attr)
+{
+ void __user *ukey = u64_to_user_ptr(attr->dump.prev_key);
+ void __user *ubuf = u64_to_user_ptr(attr->dump.buf);
+ u32 __user *ubuf_len = u64_to_user_ptr(attr->dump.buf_len);
+ int ufd = attr->dump.map_fd;
+ struct bpf_map *map;
+ void *buf, *prev_key, *key, *value;
+ u32 value_size, elem_size, buf_len, cp_len;
+ struct fd f;
+ int err;
+ bool first_key = false;
+
+ if (CHECK_ATTR(BPF_MAP_DUMP))
+ return -EINVAL;
+
+ attr->flags = 0;
+ if (attr->dump.flags & ~BPF_F_LOCK)
+ return -EINVAL;
+
+ f = fdget(ufd);
+ map = __bpf_map_get(f);
+ if (IS_ERR(map))
+ return PTR_ERR(map);
+ if (!(map_get_sys_perms(map, f) & FMODE_CAN_READ)) {
+ err = -EPERM;
+ goto err_put;
+ }
+
+ if ((attr->dump.flags & BPF_F_LOCK) &&
+ !map_value_has_spin_lock(map)) {
+ err = -EINVAL;
+ goto err_put;
+ }
+
+ if (map->map_type == BPF_MAP_TYPE_QUEUE ||
+ map->map_type == BPF_MAP_TYPE_STACK) {
+ err = -ENOTSUPP;
+ goto err_put;
+ }
+
+ value_size = bpf_map_value_size(map);
+
+ err = get_user(buf_len, ubuf_len);
+ if (err)
+ goto err_put;
+
+ elem_size = map->key_size + value_size;
+ if (buf_len < elem_size) {
+ err = -EINVAL;
+ goto err_put;
+ }
+
+ if (ukey) {
+ prev_key = __bpf_copy_key(ukey, map->key_size);
+ if (IS_ERR(prev_key)) {
+ err = PTR_ERR(prev_key);
+ goto err_put;
+ }
+ } else {
+ prev_key = NULL;
+ first_key = true;
+ }
+
+ err = -ENOMEM;
+ buf = kmalloc(elem_size, GFP_USER | __GFP_NOWARN);
+ if (!buf)
+ goto err_put;
+
+ key = buf;
+ value = key + map->key_size;
+ for (cp_len = 0; cp_len + elem_size <= buf_len;) {
+ if (signal_pending(current)) {
+ err = -EINTR;
+ break;
+ }
+
+ rcu_read_lock();
+ err = map->ops->map_get_next_key(map, prev_key, key);
+ rcu_read_unlock();
+
+ if (err)
+ break;
+
+ err = bpf_map_copy_value(map, key, value, attr->dump.flags);
+
+ if (err == -ENOENT)
+ continue;
+ if (err)
+ goto free_buf;
+
+ if (copy_to_user(ubuf + cp_len, buf, elem_size)) {
+ err = -EFAULT;
+ goto free_buf;
+ }
+
+ prev_key = key;
+ cp_len += elem_size;
+ }
+
+ if (err == -ENOENT && cp_len)
+ err = 0;
+ if (!err && (copy_to_user(ubuf_len, &cp_len, sizeof(cp_len)) ||
+ (!first_key && copy_to_user(ukey, key, map->key_size))))
+ err = -EFAULT;
+free_buf:
+ kfree(buf);
+err_put:
+ fdput(f);
+ return err;
+}
+
#define BPF_MAP_LOOKUP_AND_DELETE_ELEM_LAST_FIELD value
static int map_lookup_and_delete_elem(union bpf_attr *attr)
@@ -2910,6 +3025,9 @@ SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, siz
case BPF_MAP_LOOKUP_AND_DELETE_ELEM:
err = map_lookup_and_delete_elem(&attr);
break;
+ case BPF_MAP_DUMP:
+ err = map_dump(&attr);
+ break;
default:
err = -EINVAL;
break;
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH v2 0/4] net: dsa: Add Vitesse VSC73xx parallel mode
From: Pawel Dembicki @ 2019-07-03 17:19 UTC (permalink / raw)
Cc: Pawel Dembicki, linus.walleij, Andrew Lunn, Vivien Didelot,
Florian Fainelli, David S. Miller, Rob Herring, Mark Rutland,
netdev, devicetree, linux-kernel
Main goal of this patch series is to add support for parallel bus in
Vitesse VSC73xx switches. Existing driver supports only SPI mode.
Second change is needed for devices in unmanaged state.
V2:
- drop changes in compatible strings
- make changes less invasive
- drop mutex in platform part and move mutex from core to spi part
- fix indentation
- fix devm_ioremap_resource result check
- add cover letter
Pawel Dembicki (4):
net: dsa: Change DT bindings for Vitesse VSC73xx switches
net: dsa: vsc73xx: Split vsc73xx driver
net: dsa: vsc73xx: add support for parallel mode
net: dsa: vsc73xx: Assert reset if iCPU is enabled
.../bindings/net/dsa/vitesse,vsc73xx.txt | 57 ++++-
drivers/net/dsa/Kconfig | 19 +-
drivers/net/dsa/Makefile | 4 +-
...tesse-vsc73xx.c => vitesse-vsc73xx-core.c} | 206 +++---------------
drivers/net/dsa/vitesse-vsc73xx-platform.c | 160 ++++++++++++++
drivers/net/dsa/vitesse-vsc73xx-spi.c | 203 +++++++++++++++++
drivers/net/dsa/vitesse-vsc73xx.h | 29 +++
7 files changed, 493 insertions(+), 185 deletions(-)
rename drivers/net/dsa/{vitesse-vsc73xx.c => vitesse-vsc73xx-core.c} (90%)
create mode 100644 drivers/net/dsa/vitesse-vsc73xx-platform.c
create mode 100644 drivers/net/dsa/vitesse-vsc73xx-spi.c
create mode 100644 drivers/net/dsa/vitesse-vsc73xx.h
--
2.20.1
^ permalink raw reply
* [PATCH v2 1/4] net: dsa: Change DT bindings for Vitesse VSC73xx switches
From: Pawel Dembicki @ 2019-07-03 17:19 UTC (permalink / raw)
Cc: Pawel Dembicki, linus.walleij, Andrew Lunn, Vivien Didelot,
Florian Fainelli, David S. Miller, Rob Herring, Mark Rutland,
netdev, devicetree, linux-kernel
In-Reply-To: <20190703171924.31801-1-paweldembicki@gmail.com>
This commit introduce how to use vsc73xx platform driver.
Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>
---
.../bindings/net/dsa/vitesse,vsc73xx.txt | 57 +++++++++++++++++--
1 file changed, 53 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/dsa/vitesse,vsc73xx.txt b/Documentation/devicetree/bindings/net/dsa/vitesse,vsc73xx.txt
index ed4710c40641..c55e0148657d 100644
--- a/Documentation/devicetree/bindings/net/dsa/vitesse,vsc73xx.txt
+++ b/Documentation/devicetree/bindings/net/dsa/vitesse,vsc73xx.txt
@@ -2,8 +2,8 @@ Vitesse VSC73xx Switches
========================
This defines device tree bindings for the Vitesse VSC73xx switch chips.
-The Vitesse company has been acquired by Microsemi and Microsemi in turn
-acquired by Microchip but retains this vendor branding.
+The Vitesse company has been acquired by Microsemi and Microsemi has
+been acquired Microchip but retains this vendor branding.
The currently supported switch chips are:
Vitesse VSC7385 SparX-G5 5+1-port Integrated Gigabit Ethernet Switch
@@ -11,8 +11,13 @@ Vitesse VSC7388 SparX-G8 8-port Integrated Gigabit Ethernet Switch
Vitesse VSC7395 SparX-G5e 5+1-port Integrated Gigabit Ethernet Switch
Vitesse VSC7398 SparX-G8e 8-port Integrated Gigabit Ethernet Switch
-The device tree node is an SPI device so it must reside inside a SPI bus
-device tree node, see spi/spi-bus.txt
+This switch could have two different management interface.
+
+If SPI interface is used, the device tree node is an SPI device so it must
+reside inside a SPI bus device tree node, see spi/spi-bus.txt
+
+If Platform driver is used, the device tree node is an platform device so it
+must reside inside a platform bus device tree node.
Required properties:
@@ -38,6 +43,7 @@ and subnodes of DSA switches.
Examples:
+SPI:
switch@0 {
compatible = "vitesse,vsc7395";
reg = <0>;
@@ -79,3 +85,46 @@ switch@0 {
};
};
};
+
+Platform:
+switch@2,0 {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ compatible = "vitesse,vsc7385";
+ reg = <0x2 0x0 0x20000>;
+ reset-gpios = <&gpio0 12 GPIO_ACTIVE_LOW>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ label = "lan1";
+ };
+ port@1 {
+ reg = <1>;
+ label = "lan2";
+ };
+ port@2 {
+ reg = <2>;
+ label = "lan3";
+ };
+ port@3 {
+ reg = <3>;
+ label = "lan4";
+ };
+ vsc: port@6 {
+ reg = <6>;
+ label = "cpu";
+ ethernet = <&enet0>;
+ phy-mode = "rgmii";
+ fixed-link {
+ speed = <1000>;
+ full-duplex;
+ pause;
+ };
+ };
+ };
+
+};
--
2.20.1
^ permalink raw reply related
* [PATCH v2 3/4] net: dsa: vsc73xx: add support for parallel mode
From: Pawel Dembicki @ 2019-07-03 17:19 UTC (permalink / raw)
Cc: Pawel Dembicki, linus.walleij, Andrew Lunn, Vivien Didelot,
Florian Fainelli, David S. Miller, Rob Herring, Mark Rutland,
netdev, devicetree, linux-kernel
In-Reply-To: <20190703171924.31801-1-paweldembicki@gmail.com>
This patch add platform part of vsc73xx driver.
It allows to use chip connected by parallel interface.
Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>
---
drivers/net/dsa/Kconfig | 8 ++
drivers/net/dsa/Makefile | 1 +
drivers/net/dsa/vitesse-vsc73xx-platform.c | 160 +++++++++++++++++++++
3 files changed, 169 insertions(+)
create mode 100644 drivers/net/dsa/vitesse-vsc73xx-platform.c
diff --git a/drivers/net/dsa/Kconfig b/drivers/net/dsa/Kconfig
index 4ab2aa09e2e4..80965808949d 100644
--- a/drivers/net/dsa/Kconfig
+++ b/drivers/net/dsa/Kconfig
@@ -116,4 +116,12 @@ config NET_DSA_VITESSE_VSC73XX_SPI
---help---
This enables support for the Vitesse VSC7385, VSC7388, VSC7395
and VSC7398 SparX integrated ethernet switches in SPI managed mode.
+
+config NET_DSA_VITESSE_VSC73XX_PLATFORM
+ tristate "Vitesse VSC7385/7388/7395/7398 Platform mode support"
+ depends on HAS_IOMEM
+ select NET_DSA_VITESSE_VSC73XX
+ ---help---
+ This enables support for the Vitesse VSC7385, VSC7388, VSC7395
+ and VSC7398 SparX integrated ethernet switches in Platform managed mode.
endmenu
diff --git a/drivers/net/dsa/Makefile b/drivers/net/dsa/Makefile
index 117bf78be211..d5e4c668ac03 100644
--- a/drivers/net/dsa/Makefile
+++ b/drivers/net/dsa/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_NET_DSA_SMSC_LAN9303) += lan9303-core.o
obj-$(CONFIG_NET_DSA_SMSC_LAN9303_I2C) += lan9303_i2c.o
obj-$(CONFIG_NET_DSA_SMSC_LAN9303_MDIO) += lan9303_mdio.o
obj-$(CONFIG_NET_DSA_VITESSE_VSC73XX) += vitesse-vsc73xx-core.o
+obj-$(CONFIG_NET_DSA_VITESSE_VSC73XX_PLATFORM) += vitesse-vsc73xx-platform.o
obj-$(CONFIG_NET_DSA_VITESSE_VSC73XX_SPI) += vitesse-vsc73xx-spi.o
obj-y += b53/
obj-y += microchip/
diff --git a/drivers/net/dsa/vitesse-vsc73xx-platform.c b/drivers/net/dsa/vitesse-vsc73xx-platform.c
new file mode 100644
index 000000000000..f41dbacd0b19
--- /dev/null
+++ b/drivers/net/dsa/vitesse-vsc73xx-platform.c
@@ -0,0 +1,160 @@
+// SPDX-License-Identifier: GPL-2.0
+/* DSA driver for:
+ * Vitesse VSC7385 SparX-G5 5+1-port Integrated Gigabit Ethernet Switch
+ * Vitesse VSC7388 SparX-G8 8-port Integrated Gigabit Ethernet Switch
+ * Vitesse VSC7395 SparX-G5e 5+1-port Integrated Gigabit Ethernet Switch
+ * Vitesse VSC7398 SparX-G8e 8-port Integrated Gigabit Ethernet Switch
+ *
+ * This driver takes control of the switch chip over Platform and
+ * configures it to route packages around when connected to a CPU port.
+ *
+ * Copyright (C) 2019 pawel Dembicki <paweldembicki@gmail.com>
+ * Based on vitesse-vsc-spi.c by:
+ * Copyright (C) 2018 Linus Wallej <linus.walleij@linaro.org>
+ * Includes portions of code from the firmware uploader by:
+ * Copyright (C) 2009 Gabor Juhos <juhosg@openwrt.org>
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+
+#include "vitesse-vsc73xx.h"
+
+#define VSC73XX_CMD_PLATFORM_BLOCK_SHIFT 14
+#define VSC73XX_CMD_PLATFORM_BLOCK_MASK 0x7
+#define VSC73XX_CMD_PLATFORM_SUBBLOCK_SHIFT 10
+#define VSC73XX_CMD_PLATFORM_SUBBLOCK_MASK 0xf
+#define VSC73XX_CMD_PLATFORM_REGISTER_SHIFT 2
+
+/**
+ * struct vsc73xx_platform - VSC73xx Platform state container
+ */
+struct vsc73xx_platform {
+ struct platform_device *pdev;
+ void __iomem *base_addr;
+ struct vsc73xx vsc;
+};
+
+static const struct vsc73xx_ops vsc73xx_platform_ops;
+
+static u32 vsc73xx_make_addr(u8 block, u8 subblock, u8 reg)
+{
+ u32 ret;
+
+ ret = (block & VSC73XX_CMD_PLATFORM_BLOCK_MASK)
+ << VSC73XX_CMD_PLATFORM_BLOCK_SHIFT;
+ ret |= (subblock & VSC73XX_CMD_PLATFORM_SUBBLOCK_MASK)
+ << VSC73XX_CMD_PLATFORM_SUBBLOCK_SHIFT;
+ ret |= reg << VSC73XX_CMD_PLATFORM_REGISTER_SHIFT;
+
+ return ret;
+}
+
+static int vsc73xx_platform_read(struct vsc73xx *vsc, u8 block, u8 subblock,
+ u8 reg, u32 *val)
+{
+ struct vsc73xx_platform *vsc_platform = vsc->priv;
+ u32 offset;
+
+ if (!vsc73xx_is_addr_valid(block, subblock))
+ return -EINVAL;
+
+ offset = vsc73xx_make_addr(block, subblock, reg);
+ *val = ioread32be(vsc_platform->base_addr + offset);
+
+ return 0;
+}
+
+static int vsc73xx_platform_write(struct vsc73xx *vsc, u8 block, u8 subblock,
+ u8 reg, u32 val)
+{
+ struct vsc73xx_platform *vsc_platform = vsc->priv;
+ u32 offset;
+
+ if (!vsc73xx_is_addr_valid(block, subblock))
+ return -EINVAL;
+
+ offset = vsc73xx_make_addr(block, subblock, reg);
+ iowrite32be(val, vsc_platform->base_addr + offset);
+
+ return 0;
+}
+
+static int vsc73xx_platform_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct vsc73xx_platform *vsc_platform;
+ struct resource *res = NULL;
+ int ret;
+
+ vsc_platform = devm_kzalloc(dev, sizeof(*vsc_platform), GFP_KERNEL);
+ if (!vsc_platform)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, vsc_platform);
+ vsc_platform->pdev = pdev;
+ vsc_platform->vsc.dev = dev;
+ vsc_platform->vsc.priv = vsc_platform;
+ vsc_platform->vsc.ops = &vsc73xx_platform_ops;
+
+ /* obtain I/O memory space */
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res) {
+ dev_err(&pdev->dev, "cannot obtain I/O memory space\n");
+ ret = -ENXIO;
+ return ret;
+ }
+
+ vsc_platform->base_addr = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(vsc_platform->base_addr)) {
+ dev_err(&pdev->dev, "cannot request I/O memory space\n");
+ ret = -ENXIO;
+ return ret;
+ }
+
+ return vsc73xx_probe(&vsc_platform->vsc);
+}
+
+static int vsc73xx_platform_remove(struct platform_device *pdev)
+{
+ struct vsc73xx_platform *vsc_platform = platform_get_drvdata(pdev);
+
+ return vsc73xx_remove(&vsc_platform->vsc);
+}
+
+static const struct vsc73xx_ops vsc73xx_platform_ops = {
+ .read = vsc73xx_platform_read,
+ .write = vsc73xx_platform_write,
+};
+
+static const struct of_device_id vsc73xx_of_match[] = {
+ {
+ .compatible = "vitesse,vsc7385",
+ },
+ {
+ .compatible = "vitesse,vsc7388",
+ },
+ {
+ .compatible = "vitesse,vsc7395",
+ },
+ {
+ .compatible = "vitesse,vsc7398",
+ },
+ { },
+};
+MODULE_DEVICE_TABLE(of, vsc73xx_of_match);
+
+static struct platform_driver vsc73xx_platform_driver = {
+ .probe = vsc73xx_platform_probe,
+ .remove = vsc73xx_platform_remove,
+ .driver = {
+ .name = "vsc73xx-platform",
+ .of_match_table = vsc73xx_of_match,
+ },
+};
+module_platform_driver(vsc73xx_platform_driver);
+
+MODULE_AUTHOR("Pawel Dembicki <paweldembicki@gmail.com>");
+MODULE_DESCRIPTION("Vitesse VSC7385/7388/7395/7398 Platform driver");
+MODULE_LICENSE("GPL v2");
--
2.20.1
^ permalink raw reply related
* [PATCH v2 4/4] net: dsa: vsc73xx: Assert reset if iCPU is enabled
From: Pawel Dembicki @ 2019-07-03 17:19 UTC (permalink / raw)
Cc: Pawel Dembicki, linus.walleij, Andrew Lunn, Vivien Didelot,
Florian Fainelli, David S. Miller, Rob Herring, Mark Rutland,
netdev, devicetree, linux-kernel
In-Reply-To: <20190703171924.31801-1-paweldembicki@gmail.com>
Driver allow to use devices with disabled iCPU only.
Some devices have pre-initialised iCPU by bootloader.
That state make switch unmanaged. This patch force reset
if device is in unmanaged state. In the result chip lost
internal firmware from RAM and it can be managed.
Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>
---
drivers/net/dsa/vitesse-vsc73xx-core.c | 36 ++++++++++++--------------
1 file changed, 17 insertions(+), 19 deletions(-)
diff --git a/drivers/net/dsa/vitesse-vsc73xx-core.c b/drivers/net/dsa/vitesse-vsc73xx-core.c
index 10063f31d9a3..4525702faf68 100644
--- a/drivers/net/dsa/vitesse-vsc73xx-core.c
+++ b/drivers/net/dsa/vitesse-vsc73xx-core.c
@@ -417,22 +417,8 @@ static int vsc73xx_detect(struct vsc73xx *vsc)
}
if (val == 0xffffffff) {
- dev_info(vsc->dev, "chip seems dead, assert reset\n");
- gpiod_set_value_cansleep(vsc->reset, 1);
- /* Reset pulse should be 20ns minimum, according to datasheet
- * table 245, so 10us should be fine
- */
- usleep_range(10, 100);
- gpiod_set_value_cansleep(vsc->reset, 0);
- /* Wait 20ms according to datasheet table 245 */
- msleep(20);
-
- ret = vsc73xx_read(vsc, VSC73XX_BLOCK_SYSTEM, 0,
- VSC73XX_ICPU_MBOX_VAL, &val);
- if (val == 0xffffffff) {
- dev_err(vsc->dev, "seems not to help, giving up\n");
- return -ENODEV;
- }
+ dev_info(vsc->dev, "chip seems dead.\n");
+ return -EAGAIN;
}
ret = vsc73xx_read(vsc, VSC73XX_BLOCK_SYSTEM, 0,
@@ -483,9 +469,8 @@ static int vsc73xx_detect(struct vsc73xx *vsc)
}
if (icpu_si_boot_en && !icpu_pi_en) {
dev_err(vsc->dev,
- "iCPU enabled boots from SI, no external memory\n");
- dev_err(vsc->dev, "no idea how to deal with this\n");
- return -ENODEV;
+ "iCPU enabled boots from PI/SI, no external memory\n");
+ return -EAGAIN;
}
if (!icpu_si_boot_en && icpu_pi_en) {
dev_err(vsc->dev,
@@ -1158,6 +1143,19 @@ int vsc73xx_probe(struct vsc73xx *vsc)
msleep(20);
ret = vsc73xx_detect(vsc);
+ if (ret == -EAGAIN) {
+ dev_err(vsc->dev,
+ "Chip seams to be out of control. Assert reset and try again.\n");
+ gpiod_set_value_cansleep(vsc->reset, 1);
+ /* Reset pulse should be 20ns minimum, according to datasheet
+ * table 245, so 10us should be fine
+ */
+ usleep_range(10, 100);
+ gpiod_set_value_cansleep(vsc->reset, 0);
+ /* Wait 20ms according to datasheet table 245 */
+ msleep(20);
+ ret = vsc73xx_detect(vsc);
+ }
if (ret) {
dev_err(dev, "no chip found (%d)\n", ret);
return -ENODEV;
--
2.20.1
^ permalink raw reply related
* [PATCH v2 2/4] net: dsa: vsc73xx: Split vsc73xx driver
From: Pawel Dembicki @ 2019-07-03 17:19 UTC (permalink / raw)
Cc: Pawel Dembicki, linus.walleij, Andrew Lunn, Vivien Didelot,
Florian Fainelli, David S. Miller, Rob Herring, Mark Rutland,
netdev, devicetree, linux-kernel
In-Reply-To: <20190703171924.31801-1-paweldembicki@gmail.com>
This driver (currently) only takes control of the switch chip over
SPI and configures it to route packages around when connected to a
CPU port. But Vitesse chip support also parallel interface.
This patch split driver into two parts: core and spi. It is required
for add support to another managing interface.
Tested-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>
---
drivers/net/dsa/Kconfig | 11 +-
drivers/net/dsa/Makefile | 3 +-
...tesse-vsc73xx.c => vitesse-vsc73xx-core.c} | 170 +--------------
drivers/net/dsa/vitesse-vsc73xx-spi.c | 203 ++++++++++++++++++
drivers/net/dsa/vitesse-vsc73xx.h | 29 +++
5 files changed, 254 insertions(+), 162 deletions(-)
rename drivers/net/dsa/{vitesse-vsc73xx.c => vitesse-vsc73xx-core.c} (91%)
create mode 100644 drivers/net/dsa/vitesse-vsc73xx-spi.c
create mode 100644 drivers/net/dsa/vitesse-vsc73xx.h
diff --git a/drivers/net/dsa/Kconfig b/drivers/net/dsa/Kconfig
index b91e78e3598f..4ab2aa09e2e4 100644
--- a/drivers/net/dsa/Kconfig
+++ b/drivers/net/dsa/Kconfig
@@ -99,8 +99,8 @@ config NET_DSA_SMSC_LAN9303_MDIO
for MDIO managed mode.
config NET_DSA_VITESSE_VSC73XX
- tristate "Vitesse VSC7385/7388/7395/7398 support"
- depends on OF && SPI
+ tristate
+ depends on OF
depends on NET_DSA
select FIXED_PHY
select VITESSE_PHY
@@ -109,4 +109,11 @@ config NET_DSA_VITESSE_VSC73XX
This enables support for the Vitesse VSC7385, VSC7388,
VSC7395 and VSC7398 SparX integrated ethernet switches.
+config NET_DSA_VITESSE_VSC73XX_SPI
+ tristate "Vitesse VSC7385/7388/7395/7398 SPI mode support"
+ depends on SPI
+ select NET_DSA_VITESSE_VSC73XX
+ ---help---
+ This enables support for the Vitesse VSC7385, VSC7388, VSC7395
+ and VSC7398 SparX integrated ethernet switches in SPI managed mode.
endmenu
diff --git a/drivers/net/dsa/Makefile b/drivers/net/dsa/Makefile
index fefb6aaa82ba..117bf78be211 100644
--- a/drivers/net/dsa/Makefile
+++ b/drivers/net/dsa/Makefile
@@ -14,7 +14,8 @@ realtek-objs := realtek-smi.o rtl8366.o rtl8366rb.o
obj-$(CONFIG_NET_DSA_SMSC_LAN9303) += lan9303-core.o
obj-$(CONFIG_NET_DSA_SMSC_LAN9303_I2C) += lan9303_i2c.o
obj-$(CONFIG_NET_DSA_SMSC_LAN9303_MDIO) += lan9303_mdio.o
-obj-$(CONFIG_NET_DSA_VITESSE_VSC73XX) += vitesse-vsc73xx.o
+obj-$(CONFIG_NET_DSA_VITESSE_VSC73XX) += vitesse-vsc73xx-core.o
+obj-$(CONFIG_NET_DSA_VITESSE_VSC73XX_SPI) += vitesse-vsc73xx-spi.o
obj-y += b53/
obj-y += microchip/
obj-y += mv88e6xxx/
diff --git a/drivers/net/dsa/vitesse-vsc73xx.c b/drivers/net/dsa/vitesse-vsc73xx-core.c
similarity index 91%
rename from drivers/net/dsa/vitesse-vsc73xx.c
rename to drivers/net/dsa/vitesse-vsc73xx-core.c
index d4780610ea8a..10063f31d9a3 100644
--- a/drivers/net/dsa/vitesse-vsc73xx.c
+++ b/drivers/net/dsa/vitesse-vsc73xx-core.c
@@ -10,10 +10,6 @@
* handling the switch in a memory-mapped manner by connecting to that external
* CPU's memory bus.
*
- * This driver (currently) only takes control of the switch chip over SPI and
- * configures it to route packages around when connected to a CPU port. The
- * chip has embedded PHYs and VLAN support so we model it using DSA.
- *
* Copyright (C) 2018 Linus Wallej <linus.walleij@linaro.org>
* Includes portions of code from the firmware uploader by:
* Copyright (C) 2009 Gabor Juhos <juhosg@openwrt.org>
@@ -24,8 +20,6 @@
#include <linux/of.h>
#include <linux/of_device.h>
#include <linux/of_mdio.h>
-#include <linux/platform_device.h>
-#include <linux/spi/spi.h>
#include <linux/bitops.h>
#include <linux/if_bridge.h>
#include <linux/etherdevice.h>
@@ -34,6 +28,8 @@
#include <linux/random.h>
#include <net/dsa.h>
+#include "vitesse-vsc73xx.h"
+
#define VSC73XX_BLOCK_MAC 0x1 /* Subblocks 0-4, 6 (CPU port) */
#define VSC73XX_BLOCK_ANALYZER 0x2 /* Only subblock 0 */
#define VSC73XX_BLOCK_MII 0x3 /* Subblocks 0 and 1 */
@@ -255,13 +251,6 @@
#define VSC73XX_GLORESET_PHY_RESET BIT(1)
#define VSC73XX_GLORESET_MASTER_RESET BIT(0)
-#define VSC73XX_CMD_MODE_READ 0
-#define VSC73XX_CMD_MODE_WRITE 1
-#define VSC73XX_CMD_MODE_SHIFT 4
-#define VSC73XX_CMD_BLOCK_SHIFT 5
-#define VSC73XX_CMD_BLOCK_MASK 0x7
-#define VSC73XX_CMD_SUBBLOCK_MASK 0xf
-
#define VSC7385_CLOCK_DELAY ((3 << 4) | 3)
#define VSC7385_CLOCK_DELAY_MASK ((3 << 4) | 3)
@@ -274,20 +263,6 @@
VSC73XX_ICPU_CTRL_CLK_EN | \
VSC73XX_ICPU_CTRL_SRST)
-/**
- * struct vsc73xx - VSC73xx state container
- */
-struct vsc73xx {
- struct device *dev;
- struct gpio_desc *reset;
- struct spi_device *spi;
- struct dsa_switch *ds;
- struct gpio_chip gc;
- u16 chipid;
- u8 addr[ETH_ALEN];
- struct mutex lock; /* Protects SPI traffic */
-};
-
#define IS_7385(a) ((a)->chipid == VSC73XX_CHIPID_ID_7385)
#define IS_7388(a) ((a)->chipid == VSC73XX_CHIPID_ID_7388)
#define IS_7395(a) ((a)->chipid == VSC73XX_CHIPID_ID_7395)
@@ -365,7 +340,7 @@ static const struct vsc73xx_counter vsc73xx_tx_counters[] = {
{ 29, "TxQoSClass3" }, /* non-standard counter */
};
-static int vsc73xx_is_addr_valid(u8 block, u8 subblock)
+int vsc73xx_is_addr_valid(u8 block, u8 subblock)
{
switch (block) {
case VSC73XX_BLOCK_MAC:
@@ -396,96 +371,18 @@ static int vsc73xx_is_addr_valid(u8 block, u8 subblock)
return 0;
}
-
-static u8 vsc73xx_make_addr(u8 mode, u8 block, u8 subblock)
-{
- u8 ret;
-
- ret = (block & VSC73XX_CMD_BLOCK_MASK) << VSC73XX_CMD_BLOCK_SHIFT;
- ret |= (mode & 1) << VSC73XX_CMD_MODE_SHIFT;
- ret |= subblock & VSC73XX_CMD_SUBBLOCK_MASK;
-
- return ret;
-}
+EXPORT_SYMBOL(vsc73xx_is_addr_valid);
static int vsc73xx_read(struct vsc73xx *vsc, u8 block, u8 subblock, u8 reg,
u32 *val)
{
- struct spi_transfer t[2];
- struct spi_message m;
- u8 cmd[4];
- u8 buf[4];
- int ret;
-
- if (!vsc73xx_is_addr_valid(block, subblock))
- return -EINVAL;
-
- spi_message_init(&m);
-
- memset(&t, 0, sizeof(t));
-
- t[0].tx_buf = cmd;
- t[0].len = sizeof(cmd);
- spi_message_add_tail(&t[0], &m);
-
- t[1].rx_buf = buf;
- t[1].len = sizeof(buf);
- spi_message_add_tail(&t[1], &m);
-
- cmd[0] = vsc73xx_make_addr(VSC73XX_CMD_MODE_READ, block, subblock);
- cmd[1] = reg;
- cmd[2] = 0;
- cmd[3] = 0;
-
- mutex_lock(&vsc->lock);
- ret = spi_sync(vsc->spi, &m);
- mutex_unlock(&vsc->lock);
-
- if (ret)
- return ret;
-
- *val = (buf[0] << 24) | (buf[1] << 16) | (buf[2] << 8) | buf[3];
-
- return 0;
+ return vsc->ops->read(vsc, block, subblock, reg, val);
}
static int vsc73xx_write(struct vsc73xx *vsc, u8 block, u8 subblock, u8 reg,
u32 val)
{
- struct spi_transfer t[2];
- struct spi_message m;
- u8 cmd[2];
- u8 buf[4];
- int ret;
-
- if (!vsc73xx_is_addr_valid(block, subblock))
- return -EINVAL;
-
- spi_message_init(&m);
-
- memset(&t, 0, sizeof(t));
-
- t[0].tx_buf = cmd;
- t[0].len = sizeof(cmd);
- spi_message_add_tail(&t[0], &m);
-
- t[1].tx_buf = buf;
- t[1].len = sizeof(buf);
- spi_message_add_tail(&t[1], &m);
-
- cmd[0] = vsc73xx_make_addr(VSC73XX_CMD_MODE_WRITE, block, subblock);
- cmd[1] = reg;
-
- buf[0] = (val >> 24) & 0xff;
- buf[1] = (val >> 16) & 0xff;
- buf[2] = (val >> 8) & 0xff;
- buf[3] = val & 0xff;
-
- mutex_lock(&vsc->lock);
- ret = spi_sync(vsc->spi, &m);
- mutex_unlock(&vsc->lock);
-
- return ret;
+ return vsc->ops->write(vsc, block, subblock, reg, val);
}
static int vsc73xx_update_bits(struct vsc73xx *vsc, u8 block, u8 subblock,
@@ -1245,21 +1142,11 @@ static int vsc73xx_gpio_probe(struct vsc73xx *vsc)
return 0;
}
-static int vsc73xx_probe(struct spi_device *spi)
+int vsc73xx_probe(struct vsc73xx *vsc)
{
- struct device *dev = &spi->dev;
- struct vsc73xx *vsc;
+ struct device *dev = vsc->dev;
int ret;
- vsc = devm_kzalloc(dev, sizeof(*vsc), GFP_KERNEL);
- if (!vsc)
- return -ENOMEM;
-
- spi_set_drvdata(spi, vsc);
- vsc->spi = spi_dev_get(spi);
- vsc->dev = dev;
- mutex_init(&vsc->lock);
-
/* Release reset, if any */
vsc->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
if (IS_ERR(vsc->reset)) {
@@ -1270,14 +1157,6 @@ static int vsc73xx_probe(struct spi_device *spi)
/* Wait 20ms according to datasheet table 245 */
msleep(20);
- spi->mode = SPI_MODE_0;
- spi->bits_per_word = 8;
- ret = spi_setup(spi);
- if (ret < 0) {
- dev_err(dev, "spi setup failed.\n");
- return ret;
- }
-
ret = vsc73xx_detect(vsc);
if (ret) {
dev_err(dev, "no chip found (%d)\n", ret);
@@ -1321,43 +1200,16 @@ static int vsc73xx_probe(struct spi_device *spi)
return 0;
}
+EXPORT_SYMBOL(vsc73xx_probe);
-static int vsc73xx_remove(struct spi_device *spi)
+int vsc73xx_remove(struct vsc73xx *vsc)
{
- struct vsc73xx *vsc = spi_get_drvdata(spi);
-
dsa_unregister_switch(vsc->ds);
gpiod_set_value(vsc->reset, 1);
return 0;
}
-
-static const struct of_device_id vsc73xx_of_match[] = {
- {
- .compatible = "vitesse,vsc7385",
- },
- {
- .compatible = "vitesse,vsc7388",
- },
- {
- .compatible = "vitesse,vsc7395",
- },
- {
- .compatible = "vitesse,vsc7398",
- },
- { },
-};
-MODULE_DEVICE_TABLE(of, vsc73xx_of_match);
-
-static struct spi_driver vsc73xx_driver = {
- .probe = vsc73xx_probe,
- .remove = vsc73xx_remove,
- .driver = {
- .name = "vsc73xx",
- .of_match_table = vsc73xx_of_match,
- },
-};
-module_spi_driver(vsc73xx_driver);
+EXPORT_SYMBOL(vsc73xx_remove);
MODULE_AUTHOR("Linus Walleij <linus.walleij@linaro.org>");
MODULE_DESCRIPTION("Vitesse VSC7385/7388/7395/7398 driver");
diff --git a/drivers/net/dsa/vitesse-vsc73xx-spi.c b/drivers/net/dsa/vitesse-vsc73xx-spi.c
new file mode 100644
index 000000000000..e73c8fcddc9f
--- /dev/null
+++ b/drivers/net/dsa/vitesse-vsc73xx-spi.c
@@ -0,0 +1,203 @@
+// SPDX-License-Identifier: GPL-2.0
+/* DSA driver for:
+ * Vitesse VSC7385 SparX-G5 5+1-port Integrated Gigabit Ethernet Switch
+ * Vitesse VSC7388 SparX-G8 8-port Integrated Gigabit Ethernet Switch
+ * Vitesse VSC7395 SparX-G5e 5+1-port Integrated Gigabit Ethernet Switch
+ * Vitesse VSC7398 SparX-G8e 8-port Integrated Gigabit Ethernet Switch
+ *
+ * This driver takes control of the switch chip over SPI and
+ * configures it to route packages around when connected to a CPU port.
+ *
+ * Copyright (C) 2018 Linus Wallej <linus.walleij@linaro.org>
+ * Includes portions of code from the firmware uploader by:
+ * Copyright (C) 2009 Gabor Juhos <juhosg@openwrt.org>
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/spi/spi.h>
+
+#include "vitesse-vsc73xx.h"
+
+#define VSC73XX_CMD_SPI_MODE_READ 0
+#define VSC73XX_CMD_SPI_MODE_WRITE 1
+#define VSC73XX_CMD_SPI_MODE_SHIFT 4
+#define VSC73XX_CMD_SPI_BLOCK_SHIFT 5
+#define VSC73XX_CMD_SPI_BLOCK_MASK 0x7
+#define VSC73XX_CMD_SPI_SUBBLOCK_MASK 0xf
+
+/**
+ * struct vsc73xx_spi - VSC73xx SPI state container
+ */
+struct vsc73xx_spi {
+ struct spi_device *spi;
+ struct mutex lock; /* Protects SPI traffic */
+ struct vsc73xx vsc;
+};
+
+static const struct vsc73xx_ops vsc73xx_spi_ops;
+
+static u8 vsc73xx_make_addr(u8 mode, u8 block, u8 subblock)
+{
+ u8 ret;
+
+ ret =
+ (block & VSC73XX_CMD_SPI_BLOCK_MASK) << VSC73XX_CMD_SPI_BLOCK_SHIFT;
+ ret |= (mode & 1) << VSC73XX_CMD_SPI_MODE_SHIFT;
+ ret |= subblock & VSC73XX_CMD_SPI_SUBBLOCK_MASK;
+
+ return ret;
+}
+
+static int vsc73xx_spi_read(struct vsc73xx *vsc, u8 block, u8 subblock, u8 reg,
+ u32 *val)
+{
+ struct vsc73xx_spi *vsc_spi = vsc->priv;
+ struct spi_transfer t[2];
+ struct spi_message m;
+ u8 cmd[4];
+ u8 buf[4];
+ int ret;
+
+ if (!vsc73xx_is_addr_valid(block, subblock))
+ return -EINVAL;
+
+ spi_message_init(&m);
+
+ memset(&t, 0, sizeof(t));
+
+ t[0].tx_buf = cmd;
+ t[0].len = sizeof(cmd);
+ spi_message_add_tail(&t[0], &m);
+
+ t[1].rx_buf = buf;
+ t[1].len = sizeof(buf);
+ spi_message_add_tail(&t[1], &m);
+
+ cmd[0] = vsc73xx_make_addr(VSC73XX_CMD_SPI_MODE_READ, block, subblock);
+ cmd[1] = reg;
+ cmd[2] = 0;
+ cmd[3] = 0;
+
+ mutex_lock(&vsc_spi->lock);
+ ret = spi_sync(vsc_spi->spi, &m);
+ mutex_unlock(&vsc_spi->lock);
+
+ if (ret)
+ return ret;
+
+ *val = (buf[0] << 24) | (buf[1] << 16) | (buf[2] << 8) | buf[3];
+
+ return 0;
+}
+
+static int vsc73xx_spi_write(struct vsc73xx *vsc, u8 block, u8 subblock, u8 reg,
+ u32 val)
+{
+ struct vsc73xx_spi *vsc_spi = vsc->priv;
+ struct spi_transfer t[2];
+ struct spi_message m;
+ u8 cmd[2];
+ u8 buf[4];
+ int ret;
+
+ if (!vsc73xx_is_addr_valid(block, subblock))
+ return -EINVAL;
+
+ spi_message_init(&m);
+
+ memset(&t, 0, sizeof(t));
+
+ t[0].tx_buf = cmd;
+ t[0].len = sizeof(cmd);
+ spi_message_add_tail(&t[0], &m);
+
+ t[1].tx_buf = buf;
+ t[1].len = sizeof(buf);
+ spi_message_add_tail(&t[1], &m);
+
+ cmd[0] = vsc73xx_make_addr(VSC73XX_CMD_SPI_MODE_WRITE, block, subblock);
+ cmd[1] = reg;
+
+ buf[0] = (val >> 24) & 0xff;
+ buf[1] = (val >> 16) & 0xff;
+ buf[2] = (val >> 8) & 0xff;
+ buf[3] = val & 0xff;
+
+ mutex_lock(&vsc_spi->lock);
+ ret = spi_sync(vsc_spi->spi, &m);
+ mutex_unlock(&vsc_spi->lock);
+
+ return ret;
+}
+
+static int vsc73xx_spi_probe(struct spi_device *spi)
+{
+ struct device *dev = &spi->dev;
+ struct vsc73xx_spi *vsc_spi;
+ int ret;
+
+ vsc_spi = devm_kzalloc(dev, sizeof(*vsc_spi), GFP_KERNEL);
+ if (!vsc_spi)
+ return -ENOMEM;
+
+ spi_set_drvdata(spi, vsc_spi);
+ vsc_spi->spi = spi_dev_get(spi);
+ vsc_spi->vsc.dev = dev;
+ vsc_spi->vsc.priv = vsc_spi;
+ vsc_spi->vsc.ops = &vsc73xx_spi_ops;
+ mutex_init(&vsc_spi->lock);
+
+ spi->mode = SPI_MODE_0;
+ spi->bits_per_word = 8;
+ ret = spi_setup(spi);
+ if (ret < 0) {
+ dev_err(dev, "spi setup failed.\n");
+ return ret;
+ }
+
+ return vsc73xx_probe(&vsc_spi->vsc);
+}
+
+static int vsc73xx_spi_remove(struct spi_device *spi)
+{
+ struct vsc73xx_spi *vsc_spi = spi_get_drvdata(spi);
+
+ return vsc73xx_remove(&vsc_spi->vsc);
+}
+
+static const struct vsc73xx_ops vsc73xx_spi_ops = {
+ .read = vsc73xx_spi_read,
+ .write = vsc73xx_spi_write,
+};
+
+static const struct of_device_id vsc73xx_of_match[] = {
+ {
+ .compatible = "vitesse,vsc7385",
+ },
+ {
+ .compatible = "vitesse,vsc7388",
+ },
+ {
+ .compatible = "vitesse,vsc7395",
+ },
+ {
+ .compatible = "vitesse,vsc7398",
+ },
+ { },
+};
+MODULE_DEVICE_TABLE(of, vsc73xx_of_match);
+
+static struct spi_driver vsc73xx_spi_driver = {
+ .probe = vsc73xx_spi_probe,
+ .remove = vsc73xx_spi_remove,
+ .driver = {
+ .name = "vsc73xx-spi",
+ .of_match_table = vsc73xx_of_match,
+ },
+};
+module_spi_driver(vsc73xx_spi_driver);
+
+MODULE_AUTHOR("Linus Walleij <linus.walleij@linaro.org>");
+MODULE_DESCRIPTION("Vitesse VSC7385/7388/7395/7398 SPI driver");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/net/dsa/vitesse-vsc73xx.h b/drivers/net/dsa/vitesse-vsc73xx.h
new file mode 100644
index 000000000000..7478f8d4e0a9
--- /dev/null
+++ b/drivers/net/dsa/vitesse-vsc73xx.h
@@ -0,0 +1,29 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#include <linux/device.h>
+#include <linux/etherdevice.h>
+#include <linux/gpio/driver.h>
+
+/**
+ * struct vsc73xx - VSC73xx state container
+ */
+struct vsc73xx {
+ struct device *dev;
+ struct gpio_desc *reset;
+ struct dsa_switch *ds;
+ struct gpio_chip gc;
+ u16 chipid;
+ u8 addr[ETH_ALEN];
+ const struct vsc73xx_ops *ops;
+ void *priv;
+};
+
+struct vsc73xx_ops {
+ int (*read)(struct vsc73xx *vsc, u8 block, u8 subblock, u8 reg,
+ u32 *val);
+ int (*write)(struct vsc73xx *vsc, u8 block, u8 subblock, u8 reg,
+ u32 val);
+};
+
+int vsc73xx_is_addr_valid(u8 block, u8 subblock);
+int vsc73xx_probe(struct vsc73xx *vsc);
+int vsc73xx_remove(struct vsc73xx *vsc);
--
2.20.1
^ permalink raw reply related
* Re: [PATCH v6 net-next 1/5] xdp: allow same allocator usage
From: Jesper Dangaard Brouer @ 2019-07-03 17:40 UTC (permalink / raw)
To: Ivan Khoronzhuk
Cc: grygorii.strashko, hawk, davem, ast, linux-kernel, linux-omap,
xdp-newbies, ilias.apalodimas, netdev, daniel, jakub.kicinski,
john.fastabend, brouer
In-Reply-To: <20190703101903.8411-2-ivan.khoronzhuk@linaro.org>
On Wed, 3 Jul 2019 13:18:59 +0300
Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote:
> First of all, it is an absolute requirement that each RX-queue have
> their own page_pool object/allocator. And this change is intendant
> to handle special case, where a single RX-queue can receive packets
> from two different net_devices.
>
> In order to protect against using same allocator for 2 different rx
> queues, add queue_index to xdp_mem_allocator to catch the obvious
> mistake where queue_index mismatch, as proposed by Jesper Dangaard
> Brouer.
>
> Adding this on xdp allocator level allows drivers with such dependency
> change the allocators w/o modifications.
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> ---
> include/net/xdp_priv.h | 2 ++
> net/core/xdp.c | 55 ++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 57 insertions(+)
>
> diff --git a/include/net/xdp_priv.h b/include/net/xdp_priv.h
> index 6a8cba6ea79a..9858a4057842 100644
> --- a/include/net/xdp_priv.h
> +++ b/include/net/xdp_priv.h
> @@ -18,6 +18,8 @@ struct xdp_mem_allocator {
> struct rcu_head rcu;
> struct delayed_work defer_wq;
> unsigned long defer_warn;
> + unsigned long refcnt;
> + u32 queue_index;
> };
I don't like this approach, because I think we need to extend struct
xdp_mem_allocator with a net_device pointer, for doing dev_hold(), to
correctly handle lifetime issues. (As I tried to explain previously).
This will be much harder after this change, which is why I proposed the
other patch.
> #endif /* __LINUX_NET_XDP_PRIV_H__ */
> diff --git a/net/core/xdp.c b/net/core/xdp.c
> index 829377cc83db..4f0ddbb3717a 100644
> --- a/net/core/xdp.c
> +++ b/net/core/xdp.c
> @@ -98,6 +98,18 @@ static bool __mem_id_disconnect(int id, bool force)
> WARN(1, "Request remove non-existing id(%d), driver bug?", id);
> return true;
> }
> +
> + /* to avoid calling hash lookup twice, decrement refcnt here till it
> + * reaches zero, then it can be called from workqueue afterwards.
> + */
> + if (xa->refcnt)
> + xa->refcnt--;
> +
> + if (xa->refcnt) {
> + mutex_unlock(&mem_id_lock);
> + return true;
> + }
> +
> xa->disconnect_cnt++;
>
> /* Detects in-flight packet-pages for page_pool */
> @@ -312,6 +324,33 @@ static bool __is_supported_mem_type(enum xdp_mem_type type)
> return true;
> }
>
> +static struct xdp_mem_allocator *xdp_allocator_find(void *allocator)
> +{
> + struct xdp_mem_allocator *xae, *xa = NULL;
> + struct rhashtable_iter iter;
> +
> + if (!allocator)
> + return xa;
> +
> + rhashtable_walk_enter(mem_id_ht, &iter);
> + do {
> + rhashtable_walk_start(&iter);
> +
> + while ((xae = rhashtable_walk_next(&iter)) && !IS_ERR(xae)) {
> + if (xae->allocator == allocator) {
> + xa = xae;
> + break;
> + }
> + }
> +
> + rhashtable_walk_stop(&iter);
> +
> + } while (xae == ERR_PTR(-EAGAIN));
> + rhashtable_walk_exit(&iter);
> +
> + return xa;
> +}
> +
> int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
> enum xdp_mem_type type, void *allocator)
> {
> @@ -347,6 +386,20 @@ int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
> }
> }
>
> + mutex_lock(&mem_id_lock);
> + xdp_alloc = xdp_allocator_find(allocator);
> + if (xdp_alloc) {
> + /* One allocator per queue is supposed only */
> + if (xdp_alloc->queue_index != xdp_rxq->queue_index)
> + return -EINVAL;
> +
> + xdp_rxq->mem.id = xdp_alloc->mem.id;
> + xdp_alloc->refcnt++;
> + mutex_unlock(&mem_id_lock);
> + return 0;
> + }
> + mutex_unlock(&mem_id_lock);
> +
> xdp_alloc = kzalloc(sizeof(*xdp_alloc), gfp);
> if (!xdp_alloc)
> return -ENOMEM;
> @@ -360,6 +413,8 @@ int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
> xdp_rxq->mem.id = id;
> xdp_alloc->mem = xdp_rxq->mem;
> xdp_alloc->allocator = allocator;
> + xdp_alloc->refcnt = 1;
> + xdp_alloc->queue_index = xdp_rxq->queue_index;
>
> /* Insert allocator into ID lookup table */
> ptr = rhashtable_insert_slow(mem_id_ht, &id, &xdp_alloc->node);
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: [EXT] Re: [PATCH net-next 4/4] qed: Add devlink support for configuration attributes.
From: Jakub Kicinski @ 2019-07-03 17:42 UTC (permalink / raw)
To: Sudarsana Reddy Kalluru
Cc: Jiri Pirko, davem@davemloft.net, netdev@vger.kernel.org,
Michal Kalderon, Ariel Elior
In-Reply-To: <MN2PR18MB2528065BE3D46045F7EA1888D3FB0@MN2PR18MB2528.namprd18.prod.outlook.com>
On Wed, 3 Jul 2019 12:56:39 +0000, Sudarsana Reddy Kalluru wrote:
> Apologies for bringing this topic again. From the driver(s) code
> paths/'devlink man pages', I understood that devlink-port object is
> an entity on top of the PCI bus device. Some drivers say NFP
> represents vnics (on pci-dev) as a devlink-ports and, some represents
> (virtual?) ports on the PF/device as devlink-ports. In the case of
> Marvell NIC driver, we don't have [port] partitioning of the PCI
> device. And the config attributes are specific to PCI-device (not the
> vports/vnics of PF). Hence I didn't see a need for creating
> devlink-port objects in the system for Marvell NICs. And planning to
> add the config attributes to 'devlink-dev' object. Please let me know
> if my understanding and the proposal is ok?
I understand where you're coming from.
We want to make that judgement call on attribute-by-attribute basis.
We want consistency across vendors (and, frankly, multiple drivers of
the same vendor). If attribute looks like it belongs to the port,
rather than the entire device/ASIC operation, we should make it a port
attribute.
^ permalink raw reply
* Re: [PATCH net-next v6 09/15] ethtool: generic handlers for GET requests
From: Michal Kubecek @ 2019-07-03 17:53 UTC (permalink / raw)
To: netdev
Cc: Jiri Pirko, David Miller, Jakub Kicinski, Andrew Lunn,
Florian Fainelli, John Linville, Stephen Hemminger, Johannes Berg,
linux-kernel
In-Reply-To: <20190703142510.GA2250@nanopsycho>
On Wed, Jul 03, 2019 at 04:25:10PM +0200, Jiri Pirko wrote:
> Tue, Jul 02, 2019 at 01:50:24PM CEST, mkubecek@suse.cz wrote:
>
> [...]
>
> >+/* generic ->doit() handler for GET type requests */
> >+static int ethnl_get_doit(struct sk_buff *skb, struct genl_info *info)
>
> It is very unfortunate for review to introduce function in a patch and
> don't use it. In general, this approach is frowned upon. You should use
> whatever you introduce in the same patch. I understand it is sometimes
> hard.
It's not as if I introduced something and didn't show how to use it.
First use is in the very next patch so if you insist on reading each
patch separately without context, just combine 09/15 and 10/15 together;
the overlap is minimal (10/15 adds an entry into get_requests[]
introduced in 09/15).
I could have done that myself but the resulting patch would add over
1000 lines (also something frown upon in general) and if someone asked
if it could be split, the only honest answer I could give would be:
"Of course it should be split, it consists of two completely logically
separated parts (which are also 99% separated in code)."
> IIUC, you have one ethnl_get_doit for all possible commands, and you
Not all of them, only GET requests (and related notifications) and out
of them, only those which fit the common pattern. There will be e.g. Rx
rules and stats (maybe others) where dump request won't be iterating
through devices so that they will need at least their own dumpit
handler.
> have this ops to do cmd-specific tasks. That is quite unusual. Plus if
> you consider the complicated datastructures connected with this,
> I'm lost from the beginning :( Any particular reason form this indirection?
> I don't think any other generic netlink code does that (correct me if
> I'm wrong). The nice thing about generic netlink is the fact that
> you have separate handlers per cmd.
>
> I don't think you need these ops and indirections. For the common parts,
> just have a set of common helpers, as the other generic netlink users
> are doing. The code would be much easier to read and follow then.
As I said last time, what you suggest is going back to what I already
had in the early versions; so I have pretty good idea what the result
would look like.
I could go that way, having a separate main handler for each request
type and call common helpers from it. But as there would always be
a doit() handler, a dumpit() handler and mostly also a notification
handler, I would have to factor out the functions which are now
callbacks in struct get_request_ops anyway. To avoid too many
parameters, I would end up with structures very similar to what I have
now. (Not really "I would", the structures were already there, the only
difference was that the "request" and "data" parts were two structures
rather than one.)
So at the moment, I would have 5 functions looking almost the same as
ethnl_get_doit(), 5 functions looking almost as ethnl_get_dumpit() and
2 functions looking like ethnl_std_notify(), with the prospect of more
to be added. Any change in the logic would need to be repeated for all
of them. Moreover, you also proposed (or rather requested) to drop the
infomask concept and split the message types into multiple separate
ones. With that change, the number of almost copies would be 21 doit(),
21 dumpit() and 13 notification handlers (for now, that is).
I'm also not happy about the way typical GET and SET request processing
looks now. But I would much rather go in the opposite direction: define
relationship between message attributes and data structure members so
that most of the size estimate, data prepare, message fill and data
update functions which are all repeating the same pattern could be
replaced by universal functions doing these actions according to the
description. The direction you suggest is the direction I came from.
Seriously, I don't know what to think. Anywhere I look, return code is
checked with "if (ret < 0)" (sure, some use "if (ret)" but it's
certainly not prevalent or universally preferred, more like 1:1), now
you tell me it's wrong. Networking stack is full of simple helpers and
wrappers, yet you keep telling me simple wrappers are wrong. Networking
stack is full of abstractions and ops, you tell me it's wrong. It's
really confusing...
Michal
^ permalink raw reply
* Re: [PATCH rdma-next v2 00/13] DEVX asynchronous events
From: Leon Romanovsky @ 2019-07-03 18:04 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Doug Ledford, RDMA mailing list, Saeed Mahameed, Yishai Hadas,
linux-netdev
In-Reply-To: <20190703152902.GA582@ziepe.ca>
On Wed, Jul 03, 2019 at 12:29:03PM -0300, Jason Gunthorpe wrote:
> On Sun, Jun 30, 2019 at 07:23:21PM +0300, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro@mellanox.com>
> >
> > Changelog:
> > v1 -> v2:
> > * Added Saeed's ack to net patches
> > * Patch #2:
> > * Fix to gather user asynchronous events on top of kernel events.
> > * Patch #7:
> > * Fix obj_id to be 32 bits.
> > * Patch #8:
> > * Inline async_event_queue applicable fields into devx_async_event_file.
> > * Move to use bitfields in few places rather than flags.
> > * Shorten name of UAPI attribute.
> > * Patch #10:
> > * Use explicitly 'struct file *' instead of void *
> > * Store struct devx_async_event_file * instead of uobj * on the subscription.
> > * Drop 'is_obj_related' and use list_empty instead.
> > * Drop the temp arrays as part of the subscription API and move to simpler logic.
> > * Revise devx_cleanup_subscription() to be success oriented without
> > the is_close flag.
> > * Leave key level 1 in the tree upon bad flow to prevent a race with IRQ flow.
> > * Fix some styling notes.
> > * Patch #11:
> > * Use rcu read lock also for the un-affiliated event flow.
> > * Improve locking scheme as part of read events.
> > * Return -EIO as soon as destroyed occurred.
> > * Use a better errno as part read event failure when the buffer size
> > was too small.
> > * Upon hot unplug call wake_up_interruptible() unconditionally.
> > * Use eqe->data for affiliated events header.
> > * Fix some styling notes.
> > * Patch #12:
> > * Use rcu read lock also for the first XA layer.
> > * Patch #13:
> > * A new patch to clean up mdev usage from devx code, it can be accessed
> > from ib_dev now.
> > v0 -> v1:
> > * Fix the unbind / hot unplug flows to work properly.
> > * Fix Ref count handling on the eventfd mode in some flow.
> > * Rebased to latest rdma-next
> >
> > Thanks
> >
> > >From Yishai:
> >
> > This series enables RDMA applications that use the DEVX interface to
> > subscribe and read device asynchronous events.
> >
> > The solution is designed to allow extension of events in the future
> > without need to perform any changes in the driver code.
> >
> > To enable that few changes had been done in mlx5_core, it includes:
> > * Reading device event capabilities that are user related
> > (affiliated and un-affiliated) and set the matching mask upon
> > creating the matching EQ.
> > * Enable DEVX/mlx5_ib to register for ANY event instead of the option to
> > get some hard-coded ones.
> > * Enable DEVX/mlx5_ib to get the device raw data for CQ completion events.
> > * Enhance mlx5_core_create/destroy CQ to enable DEVX using them so that CQ
> > events will be reported as well.
> >
> > In mlx5_ib layer the below changes were done:
> > * A new DEVX API was introduced to allocate an event channel by using
> > the uverbs FD object type.
> > * Implement the FD channel operations to enable read/poo/close over it.
> > * A new DEVX API was introduced to subscribe for specific events over an
> > event channel.
> > * Manage an internal data structure over XA(s) to subscribe/dispatch events
> > over the different event channels.
> > * Use from DEVX the mlx5_core APIs to create/destroy a CQ to be able to
> > get its relevant events.
> >
> > Yishai
> >
> > Yishai Hadas (13):
> > net/mlx5: Fix mlx5_core_destroy_cq() error flow
> > net/mlx5: Use event mask based on device capabilities
> > net/mlx5: Expose the API to register for ANY event
> > net/mlx5: mlx5_core_create_cq() enhancements
> > net/mlx5: Report a CQ error event only when a handler was set
> > net/mlx5: Report EQE data upon CQ completion
> > net/mlx5: Expose device definitions for object events
> > IB/mlx5: Introduce MLX5_IB_OBJECT_DEVX_ASYNC_EVENT_FD
> > IB/mlx5: Register DEVX with mlx5_core to get async events
> > IB/mlx5: Enable subscription for device events over DEVX
> > IB/mlx5: Implement DEVX dispatching event
> > IB/mlx5: Add DEVX support for CQ events
> > IB/mlx5: DEVX cleanup mdev
>
> This looks OK now, can you please apply the net patches to the shared
> branch
Pushed to mlx5-next branch:
e4075c442876 net/mlx5: Expose device definitions for object events
4e0e2ea1886a net/mlx5: Report EQE data upon CQ completion
70a43d3fd4ef net/mlx5: Report a CQ error event only when a handler was set
38164b771947 net/mlx5: mlx5_core_create_cq() enhancements
c0670781f548 net/mlx5: Expose the API to register for ANY event
b9a7ba556207 net/mlx5: Use event mask based on device capabilities
1d49ce1e05f8 net/mlx5: Fix mlx5_core_destroy_cq() error flow
Thanks
>
> Thanks,
> Jason
^ permalink raw reply
* Re: [PATCH net-next v6 06/15] ethtool: netlink bitset handling
From: Michal Kubecek @ 2019-07-03 18:18 UTC (permalink / raw)
To: netdev
Cc: Jiri Pirko, David Miller, Jakub Kicinski, Andrew Lunn,
Florian Fainelli, John Linville, Stephen Hemminger, Johannes Berg,
linux-kernel
In-Reply-To: <20190703114933.GW2250@nanopsycho>
On Wed, Jul 03, 2019 at 01:49:33PM +0200, Jiri Pirko wrote:
> Tue, Jul 02, 2019 at 01:50:09PM CEST, mkubecek@suse.cz wrote:
> >diff --git a/Documentation/networking/ethtool-netlink.txt b/Documentation/networking/ethtool-netlink.txt
> >index 97c369aa290b..4636682c551f 100644
> >--- a/Documentation/networking/ethtool-netlink.txt
> >+++ b/Documentation/networking/ethtool-netlink.txt
> >@@ -73,6 +73,67 @@ set, the behaviour is the same as (or closer to) the behaviour before it was
> > introduced.
> >
> >
> >+Bit sets
> >+--------
> >+
> >+For short bitmaps of (reasonably) fixed length, standard NLA_BITFIELD32 type
> >+is used. For arbitrary length bitmaps, ethtool netlink uses a nested attribute
> >+with contents of one of two forms: compact (two binary bitmaps representing
> >+bit values and mask of affected bits) and bit-by-bit (list of bits identified
> >+by either index or name).
> >+
> >+Compact form: nested (bitset) atrribute contents:
> >+
> >+ ETHTOOL_A_BITSET_LIST (flag) no mask, only a list
> >+ ETHTOOL_A_BITSET_SIZE (u32) number of significant bits
> >+ ETHTOOL_A_BITSET_VALUE (binary) bitmap of bit values
> >+ ETHTOOL_A_BITSET_MASK (binary) bitmap of valid bits
> >+
> >+Value and mask must have length at least ETHTOOL_A_BITSET_SIZE bits rounded up
> >+to a multiple of 32 bits. They consist of 32-bit words in host byte order,
>
> Looks like the blocks are similar to NLA_BITFIELD32. Why don't you user
> nested array of NLA_BITFIELD32 instead?
That would mean a layout like
4 bytes of attr header
4 bytes of value
4 bytes of mask
4 bytes of attr header
4 bytes of value
4 bytes of mask
...
i.e. interleaved headers, words of value and words of mask. Having value
and mask contiguous looks cleaner to me. Also, I can quickly check the
sizes without iterating through a (potentially long) array.
> >+words ordered from least significant to most significant (i.e. the same way as
> >+bitmaps are passed with ioctl interface).
> >+
> >+For compact form, ETHTOOL_A_BITSET_SIZE and ETHTOOL_A_BITSET_VALUE are
> >+mandatory. Similar to BITFIELD32, a compact form bit set requests to set bits
>
> Double space^^
Hm, I have to learn how to tell vim not to do that with "gq".
> >+in the mask to 1 (if the bit is set in value) or 0 (if not) and preserve the
> >+rest. If ETHTOOL_A_BITSET_LIST is present, there is no mask and bitset
> >+represents a simple list of bits.
>
> Okay, that is a bit confusing. Why not to rename to something like:
> ETHTOOL_A_BITSET_NO_MASK (flag)
> ?
From the logical point of view, it's used for lists - list of link
modes, list of netdev features, list of timestamping modes etc.
The point is that in userspace requests, we sometimes want to change
some values (enable A, disable B), sometimes to define the list of
values to be set (I want (only) A, C and E to be enabled). In kernel
replies, sometimes there is a natural value/mask pairing (e.g.
advertised and supported link modes, enabled and supported WoL modes)
but often there is just one bitmap.
> >+Kernel bit set length may differ from userspace length if older application is
> >+used on newer kernel or vice versa. If userspace bitmap is longer, an error is
> >+issued only if the request actually tries to set values of some bits not
> >+recognized by kernel.
> >+
> >+Bit-by-bit form: nested (bitset) attribute contents:
> >+
> >+ ETHTOOL_A_BITSET_LIST (flag) no mask, only a list
> >+ ETHTOOL_A_BITSET_SIZE (u32) number of significant bits
> >+ ETHTOOL_A_BITSET_BIT (nested) array of bits
> >+ ETHTOOL_A_BITSET_BIT+ (nested) one bit
> >+ ETHTOOL_A_BIT_INDEX (u32) bit index (0 for LSB)
> >+ ETHTOOL_A_BIT_NAME (string) bit name
> >+ ETHTOOL_A_BIT_VALUE (flag) present if bit is set
> >+
> >+Bit size is optional for bit-by-bit form. ETHTOOL_A_BITSET_BITS nest can only
> >+contain ETHTOOL_A_BITS_BIT attributes but there can be an arbitrary number of
> >+them. A bit may be identified by its index or by its name. When used in
> >+requests, listed bits are set to 0 or 1 according to ETHTOOL_A_BIT_VALUE, the
> >+rest is preserved. A request fails if index exceeds kernel bit length or if
> >+name is not recognized.
> >+
> >+When ETHTOOL_A_BITSET_LIST flag is present, bitset is interpreted as a simple
> >+bit list. ETHTOOL_A_BIT_VALUE attributes are not used in such case. Bit list
> >+represents a bitmap with listed bits set and the rest zero.
> >+
> >+In requests, application can use either form. Form used by kernel in reply is
> >+determined by a flag in flags field of request header. Semantics of value and
> >+mask depends on the attribute. General idea is that flags control request
> >+processing, info_mask control which parts of the information are returned in
> >+"get" request and index identifies a particular subcommand or an object to
> >+which the request applies.
>
> This is quite complex and confusing. Having the same API for 2 APIs is
> odd. The API should be crystal clear, easy to use.
>
> Why can't you have 2 commands, one working with bit arrays only, one
> working with strings? Something like:
> X_GET
> ETHTOOL_A_BITS (nested)
> ETHTOOL_A_BIT_ARRAY (BITFIELD32)
> X_NAMES_GET
> ETHTOOL_A_BIT_NAMES (nested)
> ETHTOOL_A_BIT_INDEX
> ETHTOOL_A_BIT_NAME
>
> For set, you can also have multiple cmds:
> X_SET - to set many at once, by bit index
> ETHTOOL_A_BITS (nested)
> ETHTOOL_A_BIT_ARRAY (BITFIELD32)
> X_ONE_SET - to set one, by bit index
> ETHTOOL_A_BIT_INDEX
> ETHTOOL_A_BIT_VALUE
> X_ONE_SET - to set one, by name
> ETHTOOL_A_BIT_NAME
> ETHTOOL_A_BIT_VALUE
This looks as if you assume there is nothing except the bitset in the
message but that is not true. Even with your proposed breaking of
current groups, you would still have e.g. 4 bitsets in reply to netdev
features query, 3 in timestamping info GET request and often bitsets
combined with other data (e.g. WoL modes and optional WoL password).
If you wanted to further refine the message granularity to the level of
single parameters, we might be out of message type ids already.
Unless you want to forget about structured data completely and turn
everything into tunables - but that's rather scary idea.
Michal
^ permalink raw reply
* Re: [PATCH next] loopback: fix lockdep splat
From: David Miller @ 2019-07-03 18:24 UTC (permalink / raw)
To: maheshb; +Cc: netdev, edumazet, mahesh, geert
In-Reply-To: <20190703061631.84485-1-maheshb@google.com>
From: Mahesh Bandewar <maheshb@google.com>
Date: Tue, 2 Jul 2019 23:16:31 -0700
> dev_init_scheduler() and dev_activate() expect the caller to
> hold RTNL. Since we don't want blackhole device to be initialized
> per ns, we are initializing at init.
>
> [ 3.855027] Call Trace:
> [ 3.855034] dump_stack+0x67/0x95
> [ 3.855037] lockdep_rcu_suspicious+0xd5/0x110
> [ 3.855044] dev_init_scheduler+0xe3/0x120
> [ 3.855048] ? net_olddevs_init+0x60/0x60
> [ 3.855050] blackhole_netdev_init+0x45/0x6e
> [ 3.855052] do_one_initcall+0x6c/0x2fa
> [ 3.855058] ? rcu_read_lock_sched_held+0x8c/0xa0
> [ 3.855066] kernel_init_freeable+0x1e5/0x288
> [ 3.855071] ? rest_init+0x260/0x260
> [ 3.855074] kernel_init+0xf/0x180
> [ 3.855076] ? rest_init+0x260/0x260
> [ 3.855078] ret_from_fork+0x24/0x30
>
> Fixes: 4de83b88c66 ("loopback: create blackhole net device similar to loopack.")
> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Mahesh Bandewar <maheshb@google.com>
Applied.
^ permalink raw reply
* Re: KASAN: use-after-free Read in hci_cmd_timeout
From: syzbot @ 2019-07-03 18:26 UTC (permalink / raw)
To: chaitra.basappa, davem, jejb, johan.hedberg, linux-bluetooth,
linux-kernel, linux-scsi, marcel, martin.petersen,
mpt-fusionlinux.pdl, netdev, sathya.prakash,
suganath-prabu.subramani, syzkaller-bugs
In-Reply-To: <00000000000035c756058848954a@google.com>
syzbot has bisected this bug to:
commit ff92b9dd9268507e23fc10cc4341626cef50367c
Author: Suganath Prabu <suganath-prabu.subramani@broadcom.com>
Date: Thu Oct 25 14:03:40 2018 +0000
scsi: mpt3sas: Update MPI headers to support Aero controllers
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=130ac8dda00000
start commit: eca94432 Bluetooth: Fix faulty expression for minimum encr..
git tree: upstream
final crash: https://syzkaller.appspot.com/x/report.txt?x=108ac8dda00000
console output: https://syzkaller.appspot.com/x/log.txt?x=170ac8dda00000
kernel config: https://syzkaller.appspot.com/x/.config?x=f6451f0da3d42d53
dashboard link: https://syzkaller.appspot.com/bug?extid=19a9f729f05272857487
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=125b7999a00000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=176deefba00000
Reported-by: syzbot+19a9f729f05272857487@syzkaller.appspotmail.com
Fixes: ff92b9dd9268 ("scsi: mpt3sas: Update MPI headers to support Aero
controllers")
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
^ permalink raw reply
* Re: [PATCH net-next 0/3] tc-testing: Add JSON verification and simple traffic generation
From: David Miller @ 2019-07-03 18:26 UTC (permalink / raw)
To: lucasb
Cc: netdev, jhs, xiyou.wangcong, jiri, mleitner, vladbu, dcaratti,
kernel
In-Reply-To: <1562121681-9365-1-git-send-email-lucasb@mojatatu.com>
This entire patch series lacks proper signoffs.
^ permalink raw reply
* Re: [PATCH v2] net: don't warn in inet diag when IPV6 is disabled
From: David Miller @ 2019-07-03 18:26 UTC (permalink / raw)
To: stephen; +Cc: kuznet, yoshfuji, netdev
In-Reply-To: <20190702222021.28899-1-stephen@networkplumber.org>
From: Stephen Hemminger <stephen@networkplumber.org>
Date: Tue, 2 Jul 2019 15:20:21 -0700
> If IPV6 was disabled, then ss command would cause a kernel warning
> because the command was attempting to dump IPV6 socket information.
> The fix is to just remove the warning.
>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=202249
> Fixes: 432490f9d455 ("net: ip, diag -- Add diag interface for raw sockets")
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Applied, thanks.
^ 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