* Re: [PATCH v2] net: Add trace events for all receive exit points
From: Steven Rostedt @ 2018-11-12 20:32 UTC (permalink / raw)
To: Geneviève Bastien; +Cc: davem, netdev, Mathieu Desnoyers, Ingo Molnar
In-Reply-To: <20181112194405.4133-1-gbastien@versatic.net>
On Mon, 12 Nov 2018 14:44:05 -0500
Geneviève Bastien <gbastien@versatic.net> wrote:
> Trace events are already present for the receive entry points, to indicate
> how the reception entered the stack.
>
> This patch adds the corresponding exit trace events that will bound the
> reception such that all events occurring between the entry and the exit
> can be considered as part of the reception context. This greatly helps
> for dependency and root cause analyses.
>
> Without this, it is impossible to determine whether a sched_wakeup
> event following a netif_receive_skb event is the result of the packet
> reception or a simple coincidence after further processing by the
> thread.
>
> Signed-off-by: Geneviève Bastien <gbastien@versatic.net>
> CC: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> CC: Steven Rostedt <rostedt@goodmis.org>
> CC: Ingo Molnar <mingo@redhat.com>
> CC: David S. Miller <davem@davemloft.net>
> ---
> Changes in v2:
> - Add the return value to tracepoints where applicable
> - Verify if tracepoint is enabled before walking list in
> netif_receive_skb_list
> ---
> include/trace/events/net.h | 78 ++++++++++++++++++++++++++++++++++++++
> net/core/dev.c | 38 ++++++++++++++++---
> 2 files changed, 110 insertions(+), 6 deletions(-)
>
> diff --git a/include/trace/events/net.h b/include/trace/events/net.h
> index 00aa72ce0e7c..cff1a7b9d0bb 100644
> --- a/include/trace/events/net.h
> +++ b/include/trace/events/net.h
> @@ -117,6 +117,42 @@ DECLARE_EVENT_CLASS(net_dev_template,
> __get_str(name), __entry->skbaddr, __entry->len)
> )
>
> +DECLARE_EVENT_CLASS(net_dev_template_return,
> +
> + TP_PROTO(struct sk_buff *skb, int ret),
> +
> + TP_ARGS(skb, ret),
> +
> + TP_STRUCT__entry(
> + __field(void *, skbaddr)
> + __field(int, ret)
> + ),
> +
> + TP_fast_assign(
> + __entry->skbaddr = skb;
> + __entry->ret = ret;
> + ),
> +
> + TP_printk("skbaddr=%p ret=%d", __entry->skbaddr, __entry->ret)
> +)
> +
> +DECLARE_EVENT_CLASS(net_dev_template_simple,
> +
> + TP_PROTO(struct sk_buff *skb),
> +
> + TP_ARGS(skb),
> +
> + TP_STRUCT__entry(
> + __field(void *, skbaddr)
> + ),
> +
> + TP_fast_assign(
> + __entry->skbaddr = skb;
> + ),
> +
> + TP_printk("skbaddr=%p", __entry->skbaddr)
> +)
> +
> DEFINE_EVENT(net_dev_template, net_dev_queue,
>
> TP_PROTO(struct sk_buff *skb),
> @@ -244,6 +280,48 @@ DEFINE_EVENT(net_dev_rx_verbose_template, netif_rx_ni_entry,
> TP_ARGS(skb)
> );
>
> +DEFINE_EVENT(net_dev_template_return, napi_gro_frags_exit,
> +
> + TP_PROTO(struct sk_buff *skb, int ret),
> +
> + TP_ARGS(skb, ret)
> +);
> +
> +DEFINE_EVENT(net_dev_template_return, napi_gro_receive_exit,
> +
> + TP_PROTO(struct sk_buff *skb, int ret),
> +
> + TP_ARGS(skb, ret)
> +);
> +
> +DEFINE_EVENT(net_dev_template_return, netif_receive_skb_exit,
> +
> + TP_PROTO(struct sk_buff *skb, int ret),
> +
> + TP_ARGS(skb, ret)
> +);
> +
> +DEFINE_EVENT(net_dev_template_simple, netif_receive_skb_list_exit,
One small nit, and I don't care enough to ask you to fix it, but others
might care. We probably should not intermix net_dev_template_simple
events within net_dev_template_return events.
But like I said, I don't really care, it's more cosmetic than anything
else.
For the tracing side:
Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
-- Steve
> +
> + TP_PROTO(struct sk_buff *skb),
> +
> + TP_ARGS(skb)
> +);
> +
> +DEFINE_EVENT(net_dev_template_return, netif_rx_exit,
> +
> + TP_PROTO(struct sk_buff *skb, int ret),
> +
> + TP_ARGS(skb, ret)
> +);
> +
> +DEFINE_EVENT(net_dev_template_return, netif_rx_ni_exit,
> +
> + TP_PROTO(struct sk_buff *skb, int ret),
> +
> + TP_ARGS(skb, ret)
> +);
> +
> #endif /* _TRACE_NET_H */
>
>
^ permalink raw reply
* Re: [PATCH bpf-next] bpftool: make libbfd optional
From: Jakub Kicinski @ 2018-11-12 20:35 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: Stanislav Fomichev, netdev, ast, daniel, quentin.monnet
In-Reply-To: <20181112195827.l5xz5qko7l6leqhp@mini-arch>
On Mon, 12 Nov 2018 11:58:27 -0800, Stanislav Fomichev wrote:
> On 11/12, Jakub Kicinski wrote:
> > On Mon, 12 Nov 2018 10:53:28 -0800, Stanislav Fomichev wrote:
> > > diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
> > > index 61d82020af58..ec1bc2ae3c71 100644
> > > --- a/tools/bpf/bpftool/main.h
> > > +++ b/tools/bpf/bpftool/main.h
> > > @@ -147,8 +147,19 @@ int prog_parse_fd(int *argc, char ***argv);
> > > int map_parse_fd(int *argc, char ***argv);
> > > int map_parse_fd_and_info(int *argc, char ***argv, void *info, __u32 *info_len);
> > >
> > > +#ifdef HAVE_LIBBFD_SUPPORT
> > > void disasm_print_insn(unsigned char *image, ssize_t len, int opcodes,
> > > const char *arch, const char *disassembler_options);
> > > +void disasm_init(void);
> > > +#else
> > > +static inline
> > > +void disasm_print_insn(unsigned char *image, ssize_t len, int opcodes,
> > > + const char *arch, const char *disassembler_options)
> > > +{
> > > + p_err("No libbfd support");
> > > +}
> >
> > I think an error per instruction is a bit much, could we make sure we
> > error out earlier?
>
> I can return int from disasm_print_insn as an indication to
> stop/continue. Let me know if you have better ideas, will post v2 later
> today.
Why not simply:
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 5302ee282409..44edf6705ad2 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -466,6 +466,10 @@ static int do_dump(int argc, char **argv)
int fd;
if (is_prefix(*argv, "jited")) {
+#ifndef HAVE_LIBBFD_SUPPORT
+ p_err("No libbfd support");
+ return -1;
+#endif
member_len = &info.jited_prog_len;
member_ptr = &info.jited_prog_insns;
} else if (is_prefix(*argv, "xlated")) {
Perhaps wrapped into some helper in the header to avoid the yuckiness.
^ permalink raw reply related
* Re: [PATCH v2] net: Add trace events for all receive exit points
From: Mathieu Desnoyers @ 2018-11-12 20:37 UTC (permalink / raw)
To: Geneviève Bastien; +Cc: David S. Miller, netdev, rostedt, Ingo Molnar
In-Reply-To: <2134979231.4006.1542054055701.JavaMail.zimbra@efficios.com>
----- On Nov 12, 2018, at 3:20 PM, Mathieu Desnoyers mathieu.desnoyers@efficios.com wrote:
> ----- On Nov 12, 2018, at 3:09 PM, Mathieu Desnoyers
> mathieu.desnoyers@efficios.com wrote:
>
>> ----- On Nov 12, 2018, at 2:44 PM, Geneviève Bastien gbastien@versatic.net
>> wrote:
>>
>>> Trace events are already present for the receive entry points, to indicate
>>> how the reception entered the stack.
>>>
>>> This patch adds the corresponding exit trace events that will bound the
>>> reception such that all events occurring between the entry and the exit
>>> can be considered as part of the reception context. This greatly helps
>>> for dependency and root cause analyses.
>>>
>>> Without this, it is impossible to determine whether a sched_wakeup
>>> event following a netif_receive_skb event is the result of the packet
>>> reception or a simple coincidence after further processing by the
>>> thread.
>>
>> As discussed over IRC, it is _possible_ to use kretprobes to instrument
>> the exit, but it is not practical. A v3 will be sent soon to clarify
>> this part of the commit message.
>>
>> Also, I wonder if we should use "net_dev_template_exit" for the event
>> class rather than "net_dev_template_return" ?
>
> Hrm, looking at this again, I notice that there is a single DEFINE_EVENT
> using net_dev_template_simple.
>
> We could simply turn netif_receive_skb_list_exit into a TRACE_EVENT(),
> remove the net_dev_template_simple, and rename the net_dev_template_return
> to net_dev_template ?
>
> It's pretty clear from the prototype that it expects a "ret" argument,
> so I don't see the need to also state it in the template name.
As you pointed out on IRC, net_dev_template already exists. So we can
use "net_dev_rx_exit_template" which is along the same lines as
its entry counterpart "net_dev_rx_verbose_template".
Thanks,
Mathieu
>
> Thanks,
>
> Mathieu
>
>>
>> Thanks,
>>
>> Mathieu
>>
>>>
>>> Signed-off-by: Geneviève Bastien <gbastien@versatic.net>
>>> CC: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
>>> CC: Steven Rostedt <rostedt@goodmis.org>
>>> CC: Ingo Molnar <mingo@redhat.com>
>>> CC: David S. Miller <davem@davemloft.net>
>>> ---
>>> Changes in v2:
>>> - Add the return value to tracepoints where applicable
>>> - Verify if tracepoint is enabled before walking list in
>>> netif_receive_skb_list
>>> ---
>>> include/trace/events/net.h | 78 ++++++++++++++++++++++++++++++++++++++
>>> net/core/dev.c | 38 ++++++++++++++++---
>>> 2 files changed, 110 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/include/trace/events/net.h b/include/trace/events/net.h
>>> index 00aa72ce0e7c..cff1a7b9d0bb 100644
>>> --- a/include/trace/events/net.h
>>> +++ b/include/trace/events/net.h
>>> @@ -117,6 +117,42 @@ DECLARE_EVENT_CLASS(net_dev_template,
>>> __get_str(name), __entry->skbaddr, __entry->len)
>>> )
>>>
>>> +DECLARE_EVENT_CLASS(net_dev_template_return,
>>> +
>>> + TP_PROTO(struct sk_buff *skb, int ret),
>>> +
>>> + TP_ARGS(skb, ret),
>>> +
>>> + TP_STRUCT__entry(
>>> + __field(void *, skbaddr)
>>> + __field(int, ret)
>>> + ),
>>> +
>>> + TP_fast_assign(
>>> + __entry->skbaddr = skb;
>>> + __entry->ret = ret;
>>> + ),
>>> +
>>> + TP_printk("skbaddr=%p ret=%d", __entry->skbaddr, __entry->ret)
>>> +)
>>> +
>>> +DECLARE_EVENT_CLASS(net_dev_template_simple,
>>> +
>>> + TP_PROTO(struct sk_buff *skb),
>>> +
>>> + TP_ARGS(skb),
>>> +
>>> + TP_STRUCT__entry(
>>> + __field(void *, skbaddr)
>>> + ),
>>> +
>>> + TP_fast_assign(
>>> + __entry->skbaddr = skb;
>>> + ),
>>> +
>>> + TP_printk("skbaddr=%p", __entry->skbaddr)
>>> +)
>>> +
>>> DEFINE_EVENT(net_dev_template, net_dev_queue,
>>>
>>> TP_PROTO(struct sk_buff *skb),
>>> @@ -244,6 +280,48 @@ DEFINE_EVENT(net_dev_rx_verbose_template,
>>> netif_rx_ni_entry,
>>> TP_ARGS(skb)
>>> );
>>>
>>> +DEFINE_EVENT(net_dev_template_return, napi_gro_frags_exit,
>>> +
>>> + TP_PROTO(struct sk_buff *skb, int ret),
>>> +
>>> + TP_ARGS(skb, ret)
>>> +);
>>> +
>>> +DEFINE_EVENT(net_dev_template_return, napi_gro_receive_exit,
>>> +
>>> + TP_PROTO(struct sk_buff *skb, int ret),
>>> +
>>> + TP_ARGS(skb, ret)
>>> +);
>>> +
>>> +DEFINE_EVENT(net_dev_template_return, netif_receive_skb_exit,
>>> +
>>> + TP_PROTO(struct sk_buff *skb, int ret),
>>> +
>>> + TP_ARGS(skb, ret)
>>> +);
>>> +
>>> +DEFINE_EVENT(net_dev_template_simple, netif_receive_skb_list_exit,
>>> +
>>> + TP_PROTO(struct sk_buff *skb),
>>> +
>>> + TP_ARGS(skb)
>>> +);
>>> +
>>> +DEFINE_EVENT(net_dev_template_return, netif_rx_exit,
>>> +
>>> + TP_PROTO(struct sk_buff *skb, int ret),
>>> +
>>> + TP_ARGS(skb, ret)
>>> +);
>>> +
>>> +DEFINE_EVENT(net_dev_template_return, netif_rx_ni_exit,
>>> +
>>> + TP_PROTO(struct sk_buff *skb, int ret),
>>> +
>>> + TP_ARGS(skb, ret)
>>> +);
>>> +
>>> #endif /* _TRACE_NET_H */
>>>
>>> /* This part must be outside protection */
>>> diff --git a/net/core/dev.c b/net/core/dev.c
>>> index 0ffcbdd55fa9..c4dc5df34abe 100644
>>> --- a/net/core/dev.c
>>> +++ b/net/core/dev.c
>>> @@ -4520,9 +4520,14 @@ static int netif_rx_internal(struct sk_buff *skb)
>>>
>>> int netif_rx(struct sk_buff *skb)
>>> {
>>> + int ret;
>>> +
>>> trace_netif_rx_entry(skb);
>>>
>>> - return netif_rx_internal(skb);
>>> + ret = netif_rx_internal(skb);
>>> + trace_netif_rx_exit(skb, ret);
>>> +
>>> + return ret;
>>> }
>>> EXPORT_SYMBOL(netif_rx);
>>>
>>> @@ -4537,6 +4542,7 @@ int netif_rx_ni(struct sk_buff *skb)
>>> if (local_softirq_pending())
>>> do_softirq();
>>> preempt_enable();
>>> + trace_netif_rx_ni_exit(skb, err);
>>>
>>> return err;
>>> }
>>> @@ -5222,9 +5228,14 @@ static void netif_receive_skb_list_internal(struct
>>> list_head *head)
>>> */
>>> int netif_receive_skb(struct sk_buff *skb)
>>> {
>>> + int ret;
>>> +
>>> trace_netif_receive_skb_entry(skb);
>>>
>>> - return netif_receive_skb_internal(skb);
>>> + ret = netif_receive_skb_internal(skb);
>>> + trace_netif_receive_skb_exit(skb, ret);
>>> +
>>> + return ret;
>>> }
>>> EXPORT_SYMBOL(netif_receive_skb);
>>>
>>> @@ -5244,9 +5255,15 @@ void netif_receive_skb_list(struct list_head *head)
>>>
>>> if (list_empty(head))
>>> return;
>>> - list_for_each_entry(skb, head, list)
>>> - trace_netif_receive_skb_list_entry(skb);
>>> + if (trace_netif_receive_skb_list_entry_enabled()) {
>>> + list_for_each_entry(skb, head, list)
>>> + trace_netif_receive_skb_list_entry(skb);
>>> + }
>>> netif_receive_skb_list_internal(head);
>>> + if (trace_netif_receive_skb_list_exit_enabled()) {
>>> + list_for_each_entry(skb, head, list)
>>> + trace_netif_receive_skb_list_exit(skb);
>>> + }
>>> }
>>> EXPORT_SYMBOL(netif_receive_skb_list);
>>>
>>> @@ -5634,12 +5651,17 @@ static gro_result_t napi_skb_finish(gro_result_t ret,
>>> struct sk_buff *skb)
>>>
>>> gro_result_t napi_gro_receive(struct napi_struct *napi, struct sk_buff *skb)
>>> {
>>> + gro_result_t ret;
>>> +
>>> skb_mark_napi_id(skb, napi);
>>> trace_napi_gro_receive_entry(skb);
>>>
>>> skb_gro_reset_offset(skb);
>>>
>>> - return napi_skb_finish(dev_gro_receive(napi, skb), skb);
>>> + ret = napi_skb_finish(dev_gro_receive(napi, skb), skb);
>>> + trace_napi_gro_receive_exit(skb, ret);
>>> +
>>> + return ret;
>>> }
>>> EXPORT_SYMBOL(napi_gro_receive);
>>>
>>> @@ -5753,6 +5775,7 @@ static struct sk_buff *napi_frags_skb(struct napi_struct
>>> *napi)
>>>
>>> gro_result_t napi_gro_frags(struct napi_struct *napi)
>>> {
>>> + gro_result_t ret;
>>> struct sk_buff *skb = napi_frags_skb(napi);
>>>
>>> if (!skb)
>>> @@ -5760,7 +5783,10 @@ gro_result_t napi_gro_frags(struct napi_struct *napi)
>>>
>>> trace_napi_gro_frags_entry(skb);
>>>
>>> - return napi_frags_finish(napi, skb, dev_gro_receive(napi, skb));
>>> + ret = napi_frags_finish(napi, skb, dev_gro_receive(napi, skb));
>>> + trace_napi_gro_frags_exit(skb, ret);
>>> +
>>> + return ret;
>>> }
>>> EXPORT_SYMBOL(napi_gro_frags);
>>>
>>> --
>>> 2.19.1
>>
>> --
>> Mathieu Desnoyers
>> EfficiOS Inc.
>> http://www.efficios.com
>
> --
> Mathieu Desnoyers
> EfficiOS Inc.
> http://www.efficios.com
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* Re: [PATCH v2] net: Add trace events for all receive exit points
From: Steven Rostedt @ 2018-11-12 20:40 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: Geneviève Bastien, David S. Miller, netdev, Ingo Molnar
In-Reply-To: <2134979231.4006.1542054055701.JavaMail.zimbra@efficios.com>
On Mon, 12 Nov 2018 15:20:55 -0500 (EST)
Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:
>
> Hrm, looking at this again, I notice that there is a single DEFINE_EVENT
> using net_dev_template_simple.
>
> We could simply turn netif_receive_skb_list_exit into a TRACE_EVENT(),
> remove the net_dev_template_simple, and rename the net_dev_template_return
> to net_dev_template ?
This too is only cosmetic and doesn't affect the code at all, because a
TRACE_EVENT() is really just:
#define TRACE_EVENT(name, proto, args, tstruct, assign, print) \
DECLARE_EVENT_CLASS(name, \
PARAMS(proto), \
PARAMS(args), \
PARAMS(tstruct), \
PARAMS(assign), \
PARAMS(print)); \
DEFINE_EVENT(name, name, PARAMS(proto), PARAMS(args));
-- Steve
>
> It's pretty clear from the prototype that it expects a "ret" argument,
> so I don't see the need to also state it in the template name.
>
>
^ permalink raw reply
* Re: [PATCH mlx5-next 00/10] Collection of ODP fixes
From: Leon Romanovsky @ 2018-11-12 20:43 UTC (permalink / raw)
To: Doug Ledford, Jason Gunthorpe
Cc: RDMA mailing list, Artemy Kovalyov, Majd Dibbiny, Moni Shoua,
Saeed Mahameed, linux-netdev
In-Reply-To: <20181108191017.21891-1-leon@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 2622 bytes --]
On Thu, Nov 08, 2018 at 09:10:07PM +0200, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@mellanox.com>
>
> Hi,
>
> This is collection of fixes to mlx5_core and mlx5_ib ODP logic.
> There are two main reasons why we decided to forward it to mlx5-next
> and not to rdma-rc or net like our other fixes.
>
> 1. We have large number of patches exactly in that area that are ready
> for submission and we don't want to create extra merge work for
> maintainers due to that. Among those future series (already ready and
> tested): internal change of mlx5_core pagefaults handling, moving ODP
> code to RDMA, change in EQ mechanism, simplification and moving SRQ QP
> to RDMA, extra fixes to mlx5_ib ODP and ODP SRQ support.
>
> 2. Most of those fixes are for old bugs and there is no rush to fix them
> right now (in -rc1/-rc2).
>
> Thanks
>
> Moni Shoua (10):
> net/mlx5: Release resource on error flow
> net/mlx5: Add interface to hold and release core resources
> net/mlx5: Enumerate page fault types
> IB/mlx5: Lock QP during page fault handling
> net/mlx5: Return success for PAGE_FAULT_RESUME in internal error state
> net/mlx5: Use multi threaded workqueue for page fault handling
> IB/mlx5: Improve ODP debugging messages
Applied to mlx5-next
b02394aa75e3 IB/mlx5: Improve ODP debugging messages
90290db7669b net/mlx5: Use multi threaded workqueue for page fault handling
ef90c5e9757d net/mlx5: Return success for PAGE_FAULT_RESUME in internal error state
032080ab43ac IB/mlx5: Lock QP during page fault handling
c99fefea2cc9 net/mlx5: Enumerate page fault types
27e95603f4df net/mlx5: Add interface to hold and release core resources
698114968a22 net/mlx5: Release resource on error flow
> IB/mlx5: Avoid hangs due to a missing completion
> IB/mlx5: Call PAGE_FAULT_RESUME command asynchronously
Dropped to address feedback.
> net/mlx5: Remove unused function
Dropped, because it depends on dropped commit.
Thanks
>
> drivers/infiniband/core/umem_odp.c | 14 +-
> drivers/infiniband/hw/mlx5/mlx5_ib.h | 1 +
> drivers/infiniband/hw/mlx5/mr.c | 15 +-
> drivers/infiniband/hw/mlx5/odp.c | 158 ++++++++++++++----
> drivers/net/ethernet/mellanox/mlx5/core/cmd.c | 2 +-
> drivers/net/ethernet/mellanox/mlx5/core/eq.c | 21 +--
> drivers/net/ethernet/mellanox/mlx5/core/qp.c | 20 ++-
> include/linux/mlx5/device.h | 7 +
> include/linux/mlx5/driver.h | 7 +-
> include/linux/mlx5/qp.h | 5 +
> 10 files changed, 191 insertions(+), 59 deletions(-)
>
> --
> 2.19.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: [PATCH 0/4] Add SOCFPGA System Manager
From: Lee Jones @ 2018-11-13 6:39 UTC (permalink / raw)
To: Thor Thayer
Cc: peppe.cavallaro, dinguyen, linux, alexandre.torgue, joabreu,
davem, mchehab+samsung, catalin.marinas, akpm, arnd, aisheng.dong,
linux-kernel, netdev, linux-arm-kernel
In-Reply-To: <c08bf646-bf9f-5dad-acc8-6975acd6cff3@linux.intel.com>
On Fri, 09 Nov 2018, Thor Thayer wrote:
> Hi
>
> On 10/19/18 1:16 AM, Lee Jones wrote:
> > On Wed, 17 Oct 2018, Thor Thayer wrote:
> >
> >
> > > On 10/10/2018 09:42 AM, Thor Thayer wrote:
> > > > Hi
> > > > On 09/24/2018 05:09 PM, thor.thayer@linux.intel.com wrote:
> > > > > From: Thor Thayer <thor.thayer@linux.intel.com>
> > > > >
> > > > > Add MFD driver for ARM64 SOCFPGA System Manager to steer
> > > > > System Manager calls appropriately.
> > > > > The SOCFPGA System Manager includes registers from several
> > > > > SOC peripherals.
> > > > >
> > > > > On ARM32, syscon handles this aggregated register grouping.
> > > > > Redirect System Manager calls to syscon for ARM32 SOCFPGA
> > > > > systems.
> > > > >
> > > > > The ARM64 System Manager can only be accessed from priority
> > > > > level EL3 so this new MFD driver handles the calls to EL3.
> > > > >
> > > > > Thor Thayer (4):
> > > > > mfd: altera-sysmgr: Add SOCFPGA System Manager abstraction
> > > > > ARM: socfpga_defconfig: Enable CONFIG_MTD_ALTERA_SYSMGR
> > > > > arm64: defconfig: Enable CONFIG_MTD_ALTERA_SYSMGR
> > > > > net: stmmac: socfpga: Convert to shared System Manager driver
> > > > >
> > > > > MAINTAINERS | 6 +
> > > > > arch/arm/configs/socfpga_defconfig | 1 +
> > > > > arch/arm64/configs/defconfig | 1 +
> > > > > drivers/mfd/Kconfig | 9 +
> > > > > drivers/mfd/Makefile | 1 +
> > > > > drivers/mfd/altera-sysmgr.c | 310
> > > > > +++++++++++++++++++++
> > > > > .../net/ethernet/stmicro/stmmac/dwmac-socfpga.c | 4 +-
> > > > > include/linux/mfd/altera-sysmgr.h | 113 ++++++++
> > > > > 8 files changed, 444 insertions(+), 1 deletion(-)
> > > > > create mode 100644 drivers/mfd/altera-sysmgr.c
> > > > > create mode 100644 include/linux/mfd/altera-sysmgr.h
> > > > >
> > > > Gentle ping.
> > >
> > > Gentle ping again...
> > >
> > > Any comments on this patch series?
> >
> > "Please don't send content free pings and please allow a reasonable time
> > for review. People get busy, go on holiday, attend conferences and so
> > on so unless there is some reason for urgency (like critical bug fixes)
> > please allow at least a couple of weeks for review. If there have been
> > review comments then people may be waiting for those to be addressed.
> > Sending content free pings just adds to the mail volume (if they are
> > seen at all) and if something has gone wrong you'll have to resend the
> > patches anyway so resending with any comments addressed is generally a
> > much better approach."
> >
> > In this case, the we are too late in the series to have these
> > applied. Maintainers are generally preparing their submissions for
> > the merge-window. The MFD component of this set is marked as "To
> > Review" and I will get around to it when time is more abundant.
> >
>
> Gentle ping now that the merge window is over.
>
> I'm not clear on the scolding for content free pings.
>
> Other pings I've seen just reference the patch series summary instead of
> each patch individually. It seems like pings for each patch would add more
> to the mail volume than the summary but perhaps I'm misunderstanding
> something. I'm happy to ping each patch separately if that is preferable.
Please don't ping at all. If you would like to attract attention,
please resend your patch-set with s/[PATCH]/[RESEND]/.
> Series can be found here:
> https://patchwork.kernel.org/cover/10612891/
>
> [PATCH 1/4] mfd: altera-sysmgr: Add SOCFPGA System Manager
> [PATCH 2/4] ARM: socfpga_defconfig: Enable CONFIG_MTD_ALTERA_SYSMGR
> [PATCH 3/4] arm64: defconfig: Enable CONFIG_MTD_ALTERA_SYSMGR
> [PATCH 4/4] net: stmmac: socfpga: Use shared System Manager driver
>
> Thanks,
>
> Thor
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH v2] net: Add trace events for all receive exit points
From: Mathieu Desnoyers @ 2018-11-12 20:46 UTC (permalink / raw)
To: rostedt; +Cc: Geneviève Bastien, David S. Miller, netdev, Ingo Molnar
In-Reply-To: <20181112154035.56de5e23@vmware.local.home>
----- On Nov 12, 2018, at 3:40 PM, rostedt rostedt@goodmis.org wrote:
> On Mon, 12 Nov 2018 15:20:55 -0500 (EST)
> Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:
>
>>
>> Hrm, looking at this again, I notice that there is a single DEFINE_EVENT
>> using net_dev_template_simple.
>>
>> We could simply turn netif_receive_skb_list_exit into a TRACE_EVENT(),
>> remove the net_dev_template_simple, and rename the net_dev_template_return
>> to net_dev_template ?
>
> This too is only cosmetic and doesn't affect the code at all, because a
> TRACE_EVENT() is really just:
>
> #define TRACE_EVENT(name, proto, args, tstruct, assign, print) \
> DECLARE_EVENT_CLASS(name, \
> PARAMS(proto), \
> PARAMS(args), \
> PARAMS(tstruct), \
> PARAMS(assign), \
> PARAMS(print)); \
> DEFINE_EVENT(name, name, PARAMS(proto), PARAMS(args));
>
> -- Steve
>
Of course.
I also notice that in two cases, a "gro_result_t" is implicitly cast
to "int". I usually frown upon this kind of stuff, because it's asking
for trouble if gro_result_t typedef to something else than "int" in the
future.
I would recommend going for two templates, one which takes a "int"
ret parameter, and the other a "gro_result_t" ret parameter.
Or am I being too cautious ?
Thanks,
Mathieu
>>
>> It's pretty clear from the prototype that it expects a "ret" argument,
>> so I don't see the need to also state it in the template name.
>>
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* Re: [PATCH v2] net: Add trace events for all receive exit points
From: Mathieu Desnoyers @ 2018-11-12 20:56 UTC (permalink / raw)
To: rostedt; +Cc: Geneviève Bastien, David S. Miller, netdev, Ingo Molnar
In-Reply-To: <1354786248.4048.1542055613213.JavaMail.zimbra@efficios.com>
----- On Nov 12, 2018, at 3:46 PM, Mathieu Desnoyers mathieu.desnoyers@efficios.com wrote:
> ----- On Nov 12, 2018, at 3:40 PM, rostedt rostedt@goodmis.org wrote:
>
>> On Mon, 12 Nov 2018 15:20:55 -0500 (EST)
>> Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:
>>
>>>
>>> Hrm, looking at this again, I notice that there is a single DEFINE_EVENT
>>> using net_dev_template_simple.
>>>
>>> We could simply turn netif_receive_skb_list_exit into a TRACE_EVENT(),
>>> remove the net_dev_template_simple, and rename the net_dev_template_return
>>> to net_dev_template ?
>>
>> This too is only cosmetic and doesn't affect the code at all, because a
>> TRACE_EVENT() is really just:
>>
>> #define TRACE_EVENT(name, proto, args, tstruct, assign, print) \
>> DECLARE_EVENT_CLASS(name, \
>> PARAMS(proto), \
>> PARAMS(args), \
>> PARAMS(tstruct), \
>> PARAMS(assign), \
>> PARAMS(print)); \
>> DEFINE_EVENT(name, name, PARAMS(proto), PARAMS(args));
>>
>> -- Steve
>>
>
> Of course.
>
> I also notice that in two cases, a "gro_result_t" is implicitly cast
> to "int". I usually frown upon this kind of stuff, because it's asking
> for trouble if gro_result_t typedef to something else than "int" in the
> future.
>
> I would recommend going for two templates, one which takes a "int"
> ret parameter, and the other a "gro_result_t" ret parameter.
>
> Or am I being too cautious ?
Digging further, gro_result_t maps to:
enum gro_result {
GRO_MERGED,
GRO_MERGED_FREE,
GRO_HELD,
GRO_NORMAL,
GRO_DROP,
GRO_CONSUMED,
};
typedef enum gro_result gro_result_t;
So we should add a TRACE_DEFINE_ENUM() for those.
Thanks,
Mathieu
>
> Thanks,
>
> Mathieu
>
>
>>>
>>> It's pretty clear from the prototype that it expects a "ret" argument,
>>> so I don't see the need to also state it in the template name.
>>>
>
> --
> Mathieu Desnoyers
> EfficiOS Inc.
> http://www.efficios.com
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* Re: [PATCH bpf-next] bpftool: make libbfd optional
From: Stanislav Fomichev @ 2018-11-12 21:30 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: Stanislav Fomichev, netdev, ast, daniel, quentin.monnet
In-Reply-To: <20181112123533.19da3b36@cakuba.netronome.com>
On 11/12, Jakub Kicinski wrote:
> On Mon, 12 Nov 2018 11:58:27 -0800, Stanislav Fomichev wrote:
> > On 11/12, Jakub Kicinski wrote:
> > > On Mon, 12 Nov 2018 10:53:28 -0800, Stanislav Fomichev wrote:
> > > > diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
> > > > index 61d82020af58..ec1bc2ae3c71 100644
> > > > --- a/tools/bpf/bpftool/main.h
> > > > +++ b/tools/bpf/bpftool/main.h
> > > > @@ -147,8 +147,19 @@ int prog_parse_fd(int *argc, char ***argv);
> > > > int map_parse_fd(int *argc, char ***argv);
> > > > int map_parse_fd_and_info(int *argc, char ***argv, void *info, __u32 *info_len);
> > > >
> > > > +#ifdef HAVE_LIBBFD_SUPPORT
> > > > void disasm_print_insn(unsigned char *image, ssize_t len, int opcodes,
> > > > const char *arch, const char *disassembler_options);
> > > > +void disasm_init(void);
> > > > +#else
> > > > +static inline
> > > > +void disasm_print_insn(unsigned char *image, ssize_t len, int opcodes,
> > > > + const char *arch, const char *disassembler_options)
> > > > +{
> > > > + p_err("No libbfd support");
> > > > +}
> > >
> > > I think an error per instruction is a bit much, could we make sure we
> > > error out earlier?
> >
> > I can return int from disasm_print_insn as an indication to
> > stop/continue. Let me know if you have better ideas, will post v2 later
> > today.
>
> Why not simply:
Makes sense. I think I can actually repurpose disasm_init for that (the
function I added to wrap bfd_init, I think I can move the callsite here
without any problems).
> diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
> index 5302ee282409..44edf6705ad2 100644
> --- a/tools/bpf/bpftool/prog.c
> +++ b/tools/bpf/bpftool/prog.c
> @@ -466,6 +466,10 @@ static int do_dump(int argc, char **argv)
> int fd;
>
> if (is_prefix(*argv, "jited")) {
> +#ifndef HAVE_LIBBFD_SUPPORT
> + p_err("No libbfd support");
> + return -1;
> +#endif
> member_len = &info.jited_prog_len;
> member_ptr = &info.jited_prog_insns;
> } else if (is_prefix(*argv, "xlated")) {
>
> Perhaps wrapped into some helper in the header to avoid the yuckiness.
^ permalink raw reply
* [PATCH bpf-next v2] bpftool: make libbfd optional
From: Stanislav Fomichev @ 2018-11-12 21:44 UTC (permalink / raw)
To: netdev, ast, daniel, jakub.kicinski, quentin.monnet; +Cc: Stanislav Fomichev
Make it possible to build bpftool without libbfd. libbfd and libopcodes are
typically provided in dev/dbg packages (binutils-dev in debian) which we
usually don't have installed on the fleet machines and we'd like a way to have
bpftool version that works without installing any additional packages.
This excludes support for disassembling jit-ted code and prints an error if
the user tries to use these features.
Tested by:
cat > FEATURES_DUMP.bpftool <<EOF
feature-libbfd=0
feature-disassembler-four-args=1
feature-reallocarray=0
feature-libelf=1
feature-libelf-mmap=1
feature-bpf=1
EOF
FEATURES_DUMP=$PWD/FEATURES_DUMP.bpftool make
ldd bpftool | grep libbfd
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
tools/bpf/bpftool/Makefile | 13 +++++++++++--
tools/bpf/bpftool/jit_disasm.c | 8 +++++++-
tools/bpf/bpftool/main.c | 3 ---
tools/bpf/bpftool/main.h | 14 ++++++++++++++
tools/bpf/bpftool/prog.c | 3 +++
5 files changed, 35 insertions(+), 6 deletions(-)
diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
index dac7eff4c7e5..1bea6b979082 100644
--- a/tools/bpf/bpftool/Makefile
+++ b/tools/bpf/bpftool/Makefile
@@ -53,7 +53,7 @@ ifneq ($(EXTRA_LDFLAGS),)
LDFLAGS += $(EXTRA_LDFLAGS)
endif
-LIBS = -lelf -lbfd -lopcodes $(LIBBPF)
+LIBS = -lelf $(LIBBPF)
INSTALL ?= install
RM ?= rm -f
@@ -90,7 +90,16 @@ include $(wildcard $(OUTPUT)*.d)
all: $(OUTPUT)bpftool
-SRCS = $(wildcard *.c)
+BFD_SRCS = jit_disasm.c
+
+SRCS = $(filter-out $(BFD_SRCS),$(wildcard *.c))
+
+ifeq ($(feature-libbfd),1)
+CFLAGS += -DHAVE_LIBBFD_SUPPORT
+SRCS += $(BFD_SRCS)
+LIBS += -lbfd -lopcodes
+endif
+
OBJS = $(patsubst %.c,$(OUTPUT)%.o,$(SRCS)) $(OUTPUT)disasm.o
$(OUTPUT)disasm.o: $(srctree)/kernel/bpf/disasm.c
diff --git a/tools/bpf/bpftool/jit_disasm.c b/tools/bpf/bpftool/jit_disasm.c
index c75ffd9ce2bb..b2ed5ee1af5f 100644
--- a/tools/bpf/bpftool/jit_disasm.c
+++ b/tools/bpf/bpftool/jit_disasm.c
@@ -109,7 +109,7 @@ void disasm_print_insn(unsigned char *image, ssize_t len, int opcodes,
if (inf) {
bfdf->arch_info = inf;
} else {
- p_err("No libfd support for %s", arch);
+ p_err("No libbfd support for %s", arch);
return;
}
}
@@ -183,3 +183,9 @@ void disasm_print_insn(unsigned char *image, ssize_t len, int opcodes,
bfd_close(bfdf);
}
+
+int disasm_init(void)
+{
+ bfd_init();
+ return 0;
+}
diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
index 75a3296dc0bc..5c4c1cd5a7ba 100644
--- a/tools/bpf/bpftool/main.c
+++ b/tools/bpf/bpftool/main.c
@@ -31,7 +31,6 @@
* SOFTWARE.
*/
-#include <bfd.h>
#include <ctype.h>
#include <errno.h>
#include <getopt.h>
@@ -399,8 +398,6 @@ int main(int argc, char **argv)
if (argc < 0)
usage();
- bfd_init();
-
ret = cmd_select(cmds, argc, argv, do_help);
if (json_output)
diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
index 61d82020af58..10c6c16fae29 100644
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -147,8 +147,22 @@ int prog_parse_fd(int *argc, char ***argv);
int map_parse_fd(int *argc, char ***argv);
int map_parse_fd_and_info(int *argc, char ***argv, void *info, __u32 *info_len);
+#ifdef HAVE_LIBBFD_SUPPORT
void disasm_print_insn(unsigned char *image, ssize_t len, int opcodes,
const char *arch, const char *disassembler_options);
+int disasm_init(void);
+#else
+static inline
+void disasm_print_insn(unsigned char *image, ssize_t len, int opcodes,
+ const char *arch, const char *disassembler_options)
+{
+}
+static inline int disasm_init(void)
+{
+ p_err("No libbfd support");
+ return -1;
+}
+#endif
void print_data_json(uint8_t *data, size_t len);
void print_hex_data_json(uint8_t *data, size_t len);
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 5ff5544596e7..c176e1aa66fe 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -467,6 +467,9 @@ static int do_dump(int argc, char **argv)
int fd;
if (is_prefix(*argv, "jited")) {
+ if (disasm_init())
+ return -1;
+
member_len = &info.jited_prog_len;
member_ptr = &info.jited_prog_insns;
} else if (is_prefix(*argv, "xlated")) {
--
2.19.1.930.g4563a0d9d0-goog
^ permalink raw reply related
* Re: [PATCH v5 bpf-next 2/7] libbpf: cleanup after partial failure in bpf_object__pin
From: Martin Lau @ 2018-11-12 21:53 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: netdev@vger.kernel.org, linux-kselftest@vger.kernel.org,
ast@kernel.org, daniel@iogearbox.net, shuah@kernel.org,
jakub.kicinski@netronome.com, quentin.monnet@netronome.com,
Roman Gushchin, jiong.wang@netronome.com,
bhole_prashant_q7@lab.ntt.co.jp, john.fastabend@gmail.com,
jbenc@redhat.com, treeze.taeung@gmail.com, Yonghong Song,
Okash Khawaja, sandipan@linux.vnet.ibm.com
In-Reply-To: <20181109162146.78019-3-sdf@google.com>
On Fri, Nov 09, 2018 at 08:21:41AM -0800, Stanislav Fomichev wrote:
[ ... ]
> @@ -1918,23 +2160,20 @@ void *bpf_object__priv(struct bpf_object *obj)
> }
>
> static struct bpf_program *
> -__bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
> +__bpf_program__iter(struct bpf_program *p, struct bpf_object *obj, int i)
> {
> - size_t idx;
> + ssize_t idx;
>
> if (!obj->programs)
> return NULL;
> - /* First handler */
> - if (prev == NULL)
> - return &obj->programs[0];
>
> - if (prev->obj != obj) {
> + if (p->obj != obj) {
> pr_warning("error: program handler doesn't match object\n");
> return NULL;
> }
>
> - idx = (prev - obj->programs) + 1;
> - if (idx >= obj->nr_programs)
> + idx = (p - obj->programs) + i;
> + if (idx >= obj->nr_programs || idx < 0)
> return NULL;
> return &obj->programs[idx];
> }
> @@ -1944,8 +2183,29 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
> {
> struct bpf_program *prog = prev;
>
> + if (prev == NULL)
> + return obj->programs;
> +
This patch breaks the behavior introduced in
commit eac7d84519a3 ("tools: libbpf: don't return '.text' as a program for multi-function programs"):
"Make bpf_program__next() skip over '.text' section if object file
has pseudo calls. The '.text' section is hardly a program in that
case, it's more of a storage for code of functions other than main."
For example, the userspace could have been doing:
prog = bpf_program__next(NULL, obj);
bpf_program__set_type(prog, BPF_PROG_TYPE_TRACEPOINT);
bpf_object__load(obj);
For the bpf_prog.o that has pseudo calls, after this patch in bpf-next,
the prog returned by bpf_program__next() could be in ".text" instead of
the main bpf program. The next bpf_program__set_type() has
no effect to the main program. The following bpf_object__load()
will catch user in surprise with the main bpf prog in
the wrong BPF_PROG_TYPE.
> do {
> - prog = __bpf_program__next(prog, obj);
> + prog = __bpf_program__iter(prog, obj, 1);
> + } while (prog && bpf_program__is_function_storage(prog, obj));
> +
> + return prog;
> +}
> +
> +struct bpf_program *
> +bpf_program__prev(struct bpf_program *next, struct bpf_object *obj)
> +{
> + struct bpf_program *prog = next;
> +
> + if (next == NULL) {
> + if (!obj->nr_programs)
> + return NULL;
> + return obj->programs + obj->nr_programs - 1;
> + }
> +
> + do {
> + prog = __bpf_program__iter(prog, obj, -1);
> } while (prog && bpf_program__is_function_storage(prog, obj));
>
> return prog;
^ permalink raw reply
* Re: [PATCH bpf-next v2] bpftool: make libbfd optional
From: Jakub Kicinski @ 2018-11-12 22:02 UTC (permalink / raw)
To: Stanislav Fomichev; +Cc: netdev, ast, daniel, quentin.monnet, Jiong Wang
In-Reply-To: <20181112214410.14270-1-sdf@google.com>
On Mon, 12 Nov 2018 13:44:10 -0800, Stanislav Fomichev wrote:
> Make it possible to build bpftool without libbfd. libbfd and libopcodes are
> typically provided in dev/dbg packages (binutils-dev in debian) which we
> usually don't have installed on the fleet machines and we'd like a way to have
> bpftool version that works without installing any additional packages.
> This excludes support for disassembling jit-ted code and prints an error if
> the user tries to use these features.
>
> Tested by:
> cat > FEATURES_DUMP.bpftool <<EOF
> feature-libbfd=0
> feature-disassembler-four-args=1
> feature-reallocarray=0
> feature-libelf=1
> feature-libelf-mmap=1
> feature-bpf=1
> EOF
> FEATURES_DUMP=$PWD/FEATURES_DUMP.bpftool make
> ldd bpftool | grep libbfd
>
> Signed-off-by: Stanislav Fomichev <sdf@google.com>
Seems reasonable, thanks!
Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com>
^ permalink raw reply
* Re: [PATCH v5 bpf-next 2/7] libbpf: cleanup after partial failure in bpf_object__pin
From: Stanislav Fomichev @ 2018-11-12 22:10 UTC (permalink / raw)
To: Martin Lau
Cc: Stanislav Fomichev, netdev@vger.kernel.org,
linux-kselftest@vger.kernel.org, ast@kernel.org,
daniel@iogearbox.net, shuah@kernel.org,
jakub.kicinski@netronome.com, quentin.monnet@netronome.com,
Roman Gushchin, jiong.wang@netronome.com,
bhole_prashant_q7@lab.ntt.co.jp, john.fastabend@gmail.com,
jbenc@redhat.com, treeze.taeung@gmail.com, Yonghong Song,
Okash Khawaja, "sandipan@linux
In-Reply-To: <20181112215337.5ztvb5v7cf4kzr2h@kafai-mbp.dhcp.thefacebook.com>
On 11/12, Martin Lau wrote:
> On Fri, Nov 09, 2018 at 08:21:41AM -0800, Stanislav Fomichev wrote:
> [ ... ]
> > @@ -1918,23 +2160,20 @@ void *bpf_object__priv(struct bpf_object *obj)
> > }
> >
> > static struct bpf_program *
> > -__bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
> > +__bpf_program__iter(struct bpf_program *p, struct bpf_object *obj, int i)
> > {
> > - size_t idx;
> > + ssize_t idx;
> >
> > if (!obj->programs)
> > return NULL;
> > - /* First handler */
> > - if (prev == NULL)
> > - return &obj->programs[0];
> >
> > - if (prev->obj != obj) {
> > + if (p->obj != obj) {
> > pr_warning("error: program handler doesn't match object\n");
> > return NULL;
> > }
> >
> > - idx = (prev - obj->programs) + 1;
> > - if (idx >= obj->nr_programs)
> > + idx = (p - obj->programs) + i;
> > + if (idx >= obj->nr_programs || idx < 0)
> > return NULL;
> > return &obj->programs[idx];
> > }
> > @@ -1944,8 +2183,29 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
> > {
> > struct bpf_program *prog = prev;
> >
> > + if (prev == NULL)
> > + return obj->programs;
> > +
> This patch breaks the behavior introduced in
> commit eac7d84519a3 ("tools: libbpf: don't return '.text' as a program for multi-function programs"):
> "Make bpf_program__next() skip over '.text' section if object file
> has pseudo calls. The '.text' section is hardly a program in that
> case, it's more of a storage for code of functions other than main."
>
> For example, the userspace could have been doing:
> prog = bpf_program__next(NULL, obj);
> bpf_program__set_type(prog, BPF_PROG_TYPE_TRACEPOINT);
> bpf_object__load(obj);
>
> For the bpf_prog.o that has pseudo calls, after this patch in bpf-next,
> the prog returned by bpf_program__next() could be in ".text" instead of
> the main bpf program. The next bpf_program__set_type() has
> no effect to the main program. The following bpf_object__load()
> will catch user in surprise with the main bpf prog in
> the wrong BPF_PROG_TYPE.
Will something like the following fix your concern? (plus, assuming the
same for prev):
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -2216,8 +2216,11 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
{
struct bpf_program *prog = prev;
- if (prev == NULL)
- return obj->programs;
+ if (prev == NULL) {
+ prog = obj->programs;
+ if (!prog || !bpf_program__is_function_storage(prog, obj))
+ return prog;
+ }
do {
prog = __bpf_program__iter(prog, obj, 1);
Any suggestions for a better way to do it?
> > do {
> > - prog = __bpf_program__next(prog, obj);
> > + prog = __bpf_program__iter(prog, obj, 1);
> > + } while (prog && bpf_program__is_function_storage(prog, obj));
> > +
> > + return prog;
> > +}
> > +
> > +struct bpf_program *
> > +bpf_program__prev(struct bpf_program *next, struct bpf_object *obj)
> > +{
> > + struct bpf_program *prog = next;
> > +
> > + if (next == NULL) {
> > + if (!obj->nr_programs)
> > + return NULL;
> > + return obj->programs + obj->nr_programs - 1;
> > + }
> > +
> > + do {
> > + prog = __bpf_program__iter(prog, obj, -1);
> > } while (prog && bpf_program__is_function_storage(prog, obj));
> >
> > return prog;
^ permalink raw reply
* Re: [PATCH] bpf: Remove unused variable in nsim_bpf
From: Jakub Kicinski @ 2018-11-12 22:14 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Alexei Starovoitov, Daniel Borkmann, Quentin Monnet, netdev,
linux-kernel
In-Reply-To: <20181112221042.11160-1-natechancellor@gmail.com>
On Mon, 12 Nov 2018 15:10:42 -0700, Nathan Chancellor wrote:
> Clang warns:
>
> drivers/net/netdevsim/bpf.c:557:30: error: unused variable 'state'
> [-Werror,-Wunused-variable]
> struct nsim_bpf_bound_prog *state;
> ^
> 1 error generated.
>
> The declaration should have been removed in commit b07ade27e933 ("bpf:
> pass translate() as a callback and remove its ndo_bpf subcommand").
>
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com>
^ permalink raw reply
* [iproute PATCH] man: ip-route.8: Document nexthop limit
From: Phil Sutter @ 2018-11-12 22:21 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
Add a note to 'nexthop' description stating the maximum number of
nexthops per command and pointing at 'append' command as a workaround.
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
man/man8/ip-route.8.in | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/man/man8/ip-route.8.in b/man/man8/ip-route.8.in
index a33ce1f0f4006..383178c11331e 100644
--- a/man/man8/ip-route.8.in
+++ b/man/man8/ip-route.8.in
@@ -589,6 +589,13 @@ argument lists:
route reflecting its relative bandwidth or quality.
.in -8
+The internal buffer used in iproute2 limits the maximum number of nexthops to
+be specified in one go. If only a gateway address is given, the current buffer
+size allows for 144 IPv6 nexthops and 253 IPv4 ones. If more are required, they
+may be added to the existing route using
+.B "ip route append"
+command.
+
.TP
.BI scope " SCOPE_VAL"
the scope of the destinations covered by the route prefix.
--
2.19.0
^ permalink raw reply related
* Re: [Patch net-next] net: dump more useful information in netdev_rx_csum_fault()
From: Cong Wang @ 2018-11-12 22:41 UTC (permalink / raw)
To: David Miller; +Cc: Linux Kernel Network Developers
In-Reply-To: <20181109.201630.1672707103795948980.davem@davemloft.net>
On Fri, Nov 9, 2018 at 8:16 PM David Miller <davem@davemloft.net> wrote:
>
> Didn't you move this function into net/core/skbuff.c? :-)
At the time when I created this patch, that patch didn't hit net-next yet. :)
>
> Please respin.
Sure. I will send v2, as I need to cleanup the mixed commas and
spaces too.
Thanks.
^ permalink raw reply
* [Patch net-next v2] net: dump more useful information in netdev_rx_csum_fault()
From: Cong Wang @ 2018-11-12 22:47 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang
Currently netdev_rx_csum_fault() only shows a device name,
we need more information about the skb for debugging csum
failures.
Sample output:
ens3: hw csum failure
dev features: 0x0000000000014b89
skb len=84 data_len=0 pkt_type=0 gso_size=0 gso_type=0 nr_frags=0 ip_summed=0 csum=0 csum_complete_sw=0 csum_valid=0 csum_level=0
Note, I use pr_err() just to be consistent with the existing one.
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
include/linux/netdevice.h | 5 +++--
net/core/datagram.c | 2 +-
net/core/dev.c | 11 +++++++++--
net/core/skbuff.c | 4 ++--
net/sunrpc/socklib.c | 2 +-
5 files changed, 16 insertions(+), 8 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 487fa5e0e165..5a97ffbff932 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -4342,9 +4342,10 @@ static inline bool can_checksum_protocol(netdev_features_t features,
}
#ifdef CONFIG_BUG
-void netdev_rx_csum_fault(struct net_device *dev);
+void netdev_rx_csum_fault(struct net_device *dev, struct sk_buff *skb);
#else
-static inline void netdev_rx_csum_fault(struct net_device *dev)
+static inline void netdev_rx_csum_fault(struct net_device *dev,
+ struct sk_buff *skb)
{
}
#endif
diff --git a/net/core/datagram.c b/net/core/datagram.c
index 07983b90d2bd..4bf62b1afa3b 100644
--- a/net/core/datagram.c
+++ b/net/core/datagram.c
@@ -767,7 +767,7 @@ int skb_copy_and_csum_datagram_msg(struct sk_buff *skb,
if (unlikely(skb->ip_summed == CHECKSUM_COMPLETE) &&
!skb->csum_complete_sw)
- netdev_rx_csum_fault(NULL);
+ netdev_rx_csum_fault(NULL, skb);
}
return 0;
fault:
diff --git a/net/core/dev.c b/net/core/dev.c
index bf7e0a471186..5927f6a7c301 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3091,10 +3091,17 @@ EXPORT_SYMBOL(__skb_gso_segment);
/* Take action when hardware reception checksum errors are detected. */
#ifdef CONFIG_BUG
-void netdev_rx_csum_fault(struct net_device *dev)
+void netdev_rx_csum_fault(struct net_device *dev, struct sk_buff *skb)
{
if (net_ratelimit()) {
pr_err("%s: hw csum failure\n", dev ? dev->name : "<unknown>");
+ if (dev)
+ pr_err("dev features: %pNF\n", &dev->features);
+ pr_err("skb len=%u data_len=%u pkt_type=%u gso_size=%u gso_type=%u nr_frags=%u ip_summed=%u csum=%x csum_complete_sw=%d csum_valid=%d csum_level=%u\n",
+ skb->len, skb->data_len, skb->pkt_type,
+ skb_shinfo(skb)->gso_size, skb_shinfo(skb)->gso_type,
+ skb_shinfo(skb)->nr_frags, skb->ip_summed, skb->csum,
+ skb->csum_complete_sw, skb->csum_valid, skb->csum_level);
dump_stack();
}
}
@@ -5781,7 +5788,7 @@ __sum16 __skb_gro_checksum_complete(struct sk_buff *skb)
if (likely(!sum)) {
if (unlikely(skb->ip_summed == CHECKSUM_COMPLETE) &&
!skb->csum_complete_sw)
- netdev_rx_csum_fault(skb->dev);
+ netdev_rx_csum_fault(skb->dev, skb);
}
NAPI_GRO_CB(skb)->csum = wsum;
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 396fcb3baad0..fcb1155a00ec 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -2653,7 +2653,7 @@ __sum16 __skb_checksum_complete_head(struct sk_buff *skb, int len)
if (likely(!sum)) {
if (unlikely(skb->ip_summed == CHECKSUM_COMPLETE) &&
!skb->csum_complete_sw)
- netdev_rx_csum_fault(skb->dev);
+ netdev_rx_csum_fault(skb->dev, skb);
}
if (!skb_shared(skb))
skb->csum_valid = !sum;
@@ -2673,7 +2673,7 @@ __sum16 __skb_checksum_complete(struct sk_buff *skb)
if (likely(!sum)) {
if (unlikely(skb->ip_summed == CHECKSUM_COMPLETE) &&
!skb->csum_complete_sw)
- netdev_rx_csum_fault(skb->dev);
+ netdev_rx_csum_fault(skb->dev, skb);
}
if (!skb_shared(skb)) {
diff --git a/net/sunrpc/socklib.c b/net/sunrpc/socklib.c
index 9062967575c4..7e55cfc69697 100644
--- a/net/sunrpc/socklib.c
+++ b/net/sunrpc/socklib.c
@@ -175,7 +175,7 @@ int csum_partial_copy_to_xdr(struct xdr_buf *xdr, struct sk_buff *skb)
return -1;
if (unlikely(skb->ip_summed == CHECKSUM_COMPLETE) &&
!skb->csum_complete_sw)
- netdev_rx_csum_fault(skb->dev);
+ netdev_rx_csum_fault(skb->dev, skb);
return 0;
no_checksum:
if (xdr_partial_copy_from_skb(xdr, 0, &desc, xdr_skb_read_bits) < 0)
--
2.19.1
^ permalink raw reply related
* [PATCH net-next 00/13] nfp: abm: track all Qdiscs
From: Jakub Kicinski @ 2018-11-12 22:58 UTC (permalink / raw)
To: davem; +Cc: oss-drivers, netdev, jiri, xiyou.wangcong, jhs, Jakub Kicinski
Hi!
Our Qdisc offload so far has been very simplistic. We held
and array of marking thresholds and statistics sized to the
number of PF queues. This was sufficient since the only
configuration we supported was single layer of RED Qdiscs
(on top of MQ or not, but MQ isn't really about queuing).
As we move to add more Qdiscs it's time to actually try to
track the full Qdisc hierarchy. This allows us to make sure
our offloaded configuration reflects the SW path better.
We add graft notifications to MQ and RED (PRIO already sends
them) to allow drivers offloading those to learn how Qdiscs
are linked. MQ graft gives us the obvious advantage of being
able to track when Qdiscs are shared or moved. It seems
unlikely HW would offload RED's child Qdiscs but since the
behaviour would change based on linked child we should
stop offloading REDs with modified child. RED will also
handle the child differently during reconfig when limit
parameter is set - so we have to inform the drivers about
the limit, and have them reset the child state when
appropriate.
The NFP driver will now allocate a structure to track each
Qdisc and link it to its children. We will also maintain
a shadow copy of threshold settings - to save device writes
and make it easier to apply defaults when config is
re-evaluated.
Jakub Kicinski (13):
nfp: abm: rename qdiscs -> red_qdiscs
nfp: abm: keep track of all RED thresholds
nfp: abm: track all offload-enabled qdiscs
net: sched: provide notification for graft on root
nfp: abm: remember which Qdisc is root
nfp: abm: allocate Qdisc child table
net: sched: red: offload a graft notification
net: sched: mq: offload a graft notification
nfp: abm: build full Qdisc hierarchy based on graft notifications
net: sched: red: notify drivers about RED's limit parameter
nfp: abm: reset RED's child based on limit
nfp: abm: save RED's parameters
nfp: abm: restructure Qdisc handling
drivers/net/ethernet/netronome/nfp/abm/ctrl.c | 105 +--
drivers/net/ethernet/netronome/nfp/abm/main.c | 42 +-
drivers/net/ethernet/netronome/nfp/abm/main.h | 108 ++-
.../net/ethernet/netronome/nfp/abm/qdisc.c | 712 +++++++++++++-----
include/linux/netdevice.h | 1 +
include/net/pkt_cls.h | 24 +-
net/sched/sch_api.c | 17 +
net/sched/sch_mq.c | 9 +
net/sched/sch_red.c | 18 +
9 files changed, 740 insertions(+), 296 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH net-next 01/13] nfp: abm: rename qdiscs -> red_qdiscs
From: Jakub Kicinski @ 2018-11-12 22:58 UTC (permalink / raw)
To: davem; +Cc: oss-drivers, netdev, jiri, xiyou.wangcong, jhs, Jakub Kicinski
In-Reply-To: <20181112225819.29823-1-jakub.kicinski@netronome.com>
Rename qdiscs member to red_qdiscs. One of following patches will
use the name qdiscs for tracking all qdisc types.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: John Hurley <john.hurley@netronome.com>
---
drivers/net/ethernet/netronome/nfp/abm/main.c | 11 +++---
drivers/net/ethernet/netronome/nfp/abm/main.h | 4 +-
.../net/ethernet/netronome/nfp/abm/qdisc.c | 38 ++++++++++---------
3 files changed, 29 insertions(+), 24 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/abm/main.c b/drivers/net/ethernet/netronome/nfp/abm/main.c
index 3d15de0ae271..5e749602989e 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/main.c
+++ b/drivers/net/ethernet/netronome/nfp/abm/main.c
@@ -309,9 +309,10 @@ nfp_abm_vnic_alloc(struct nfp_app *app, struct nfp_net *nn, unsigned int id)
alink->id = id;
alink->parent = TC_H_ROOT;
alink->total_queues = alink->vnic->max_rx_rings;
- alink->qdiscs = kvcalloc(alink->total_queues, sizeof(*alink->qdiscs),
- GFP_KERNEL);
- if (!alink->qdiscs) {
+ alink->red_qdiscs = kvcalloc(alink->total_queues,
+ sizeof(*alink->red_qdiscs),
+ GFP_KERNEL);
+ if (!alink->red_qdiscs) {
err = -ENOMEM;
goto err_free_alink;
}
@@ -331,7 +332,7 @@ nfp_abm_vnic_alloc(struct nfp_app *app, struct nfp_net *nn, unsigned int id)
return 0;
err_free_qdiscs:
- kvfree(alink->qdiscs);
+ kvfree(alink->red_qdiscs);
err_free_alink:
kfree(alink);
return err;
@@ -342,7 +343,7 @@ static void nfp_abm_vnic_free(struct nfp_app *app, struct nfp_net *nn)
struct nfp_abm_link *alink = nn->app_priv;
nfp_abm_kill_reprs(alink->abm, alink);
- kvfree(alink->qdiscs);
+ kvfree(alink->red_qdiscs);
kfree(alink);
}
diff --git a/drivers/net/ethernet/netronome/nfp/abm/main.h b/drivers/net/ethernet/netronome/nfp/abm/main.h
index 3774c063e419..a09090004f82 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/main.h
+++ b/drivers/net/ethernet/netronome/nfp/abm/main.h
@@ -82,7 +82,7 @@ struct nfp_red_qdisc {
* @total_queues: number of PF queues
* @parent: handle of expected parent, i.e. handle of MQ, or TC_H_ROOT
* @num_qdiscs: number of currently used qdiscs
- * @qdiscs: array of qdiscs
+ * @red_qdiscs: array of qdiscs
*/
struct nfp_abm_link {
struct nfp_abm *abm;
@@ -92,7 +92,7 @@ struct nfp_abm_link {
unsigned int total_queues;
u32 parent;
unsigned int num_qdiscs;
- struct nfp_red_qdisc *qdiscs;
+ struct nfp_red_qdisc *red_qdiscs;
};
int nfp_abm_setup_tc_red(struct net_device *netdev, struct nfp_abm_link *alink,
diff --git a/drivers/net/ethernet/netronome/nfp/abm/qdisc.c b/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
index bb05f9ee0401..abda392880e0 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
+++ b/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
@@ -18,7 +18,8 @@ __nfp_abm_reset_root(struct net_device *netdev, struct nfp_abm_link *alink,
int ret;
ret = nfp_abm_ctrl_set_all_q_lvls(alink, init_val);
- memset(alink->qdiscs, 0, sizeof(*alink->qdiscs) * alink->num_qdiscs);
+ memset(alink->red_qdiscs, 0,
+ sizeof(*alink->red_qdiscs) * alink->num_qdiscs);
alink->parent = handle;
alink->num_qdiscs = qs;
@@ -46,7 +47,8 @@ nfp_abm_red_find(struct nfp_abm_link *alink, struct tc_red_qopt_offload *opt)
else
return -EOPNOTSUPP;
- if (i >= alink->num_qdiscs || opt->handle != alink->qdiscs[i].handle)
+ if (i >= alink->num_qdiscs ||
+ opt->handle != alink->red_qdiscs[i].handle)
return -EOPNOTSUPP;
return i;
@@ -59,7 +61,7 @@ nfp_abm_red_destroy(struct net_device *netdev, struct nfp_abm_link *alink,
unsigned int i;
for (i = 0; i < alink->num_qdiscs; i++)
- if (handle == alink->qdiscs[i].handle)
+ if (handle == alink->red_qdiscs[i].handle)
break;
if (i == alink->num_qdiscs)
return;
@@ -68,7 +70,7 @@ nfp_abm_red_destroy(struct net_device *netdev, struct nfp_abm_link *alink,
nfp_abm_reset_root(netdev, alink, TC_H_ROOT, 0);
} else {
nfp_abm_ctrl_set_q_lvl(alink, i, NFP_ABM_LVL_INFINITY);
- memset(&alink->qdiscs[i], 0, sizeof(*alink->qdiscs));
+ memset(&alink->red_qdiscs[i], 0, sizeof(*alink->red_qdiscs));
}
}
@@ -139,37 +141,39 @@ nfp_abm_red_replace(struct net_device *netdev, struct nfp_abm_link *alink,
return -EINVAL;
}
/* Set the handle to try full clean up, in case IO failed */
- alink->qdiscs[i].handle = opt->handle;
+ alink->red_qdiscs[i].handle = opt->handle;
if (err)
goto err_destroy;
if (opt->parent == TC_H_ROOT)
- err = nfp_abm_ctrl_read_stats(alink, &alink->qdiscs[i].stats);
+ err = nfp_abm_ctrl_read_stats(alink,
+ &alink->red_qdiscs[i].stats);
else
err = nfp_abm_ctrl_read_q_stats(alink, i,
- &alink->qdiscs[i].stats);
+ &alink->red_qdiscs[i].stats);
if (err)
goto err_destroy;
if (opt->parent == TC_H_ROOT)
err = nfp_abm_ctrl_read_xstats(alink,
- &alink->qdiscs[i].xstats);
+ &alink->red_qdiscs[i].xstats);
else
err = nfp_abm_ctrl_read_q_xstats(alink, i,
- &alink->qdiscs[i].xstats);
+ &alink->red_qdiscs[i].xstats);
if (err)
goto err_destroy;
- alink->qdiscs[i].stats.backlog_pkts = 0;
- alink->qdiscs[i].stats.backlog_bytes = 0;
+ alink->red_qdiscs[i].stats.backlog_pkts = 0;
+ alink->red_qdiscs[i].stats.backlog_bytes = 0;
return 0;
err_destroy:
/* If the qdisc keeps on living, but we can't offload undo changes */
if (existing) {
- opt->set.qstats->qlen -= alink->qdiscs[i].stats.backlog_pkts;
+ opt->set.qstats->qlen -=
+ alink->red_qdiscs[i].stats.backlog_pkts;
opt->set.qstats->backlog -=
- alink->qdiscs[i].stats.backlog_bytes;
+ alink->red_qdiscs[i].stats.backlog_bytes;
}
nfp_abm_red_destroy(netdev, alink, opt->handle);
@@ -198,7 +202,7 @@ nfp_abm_red_stats(struct nfp_abm_link *alink, struct tc_red_qopt_offload *opt)
i = nfp_abm_red_find(alink, opt);
if (i < 0)
return i;
- prev_stats = &alink->qdiscs[i].stats;
+ prev_stats = &alink->red_qdiscs[i].stats;
if (alink->parent == TC_H_ROOT)
err = nfp_abm_ctrl_read_stats(alink, &stats);
@@ -224,7 +228,7 @@ nfp_abm_red_xstats(struct nfp_abm_link *alink, struct tc_red_qopt_offload *opt)
i = nfp_abm_red_find(alink, opt);
if (i < 0)
return i;
- prev_xstats = &alink->qdiscs[i].xstats;
+ prev_xstats = &alink->red_qdiscs[i].xstats;
if (alink->parent == TC_H_ROOT)
err = nfp_abm_ctrl_read_xstats(alink, &xstats);
@@ -267,14 +271,14 @@ nfp_abm_mq_stats(struct nfp_abm_link *alink, struct tc_mq_qopt_offload *opt)
int err;
for (i = 0; i < alink->num_qdiscs; i++) {
- if (alink->qdiscs[i].handle == TC_H_UNSPEC)
+ if (alink->red_qdiscs[i].handle == TC_H_UNSPEC)
continue;
err = nfp_abm_ctrl_read_q_stats(alink, i, &stats);
if (err)
return err;
- nfp_abm_update_stats(&stats, &alink->qdiscs[i].stats,
+ nfp_abm_update_stats(&stats, &alink->red_qdiscs[i].stats,
&opt->stats);
}
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 02/13] nfp: abm: keep track of all RED thresholds
From: Jakub Kicinski @ 2018-11-12 22:58 UTC (permalink / raw)
To: davem; +Cc: oss-drivers, netdev, jiri, xiyou.wangcong, jhs, Jakub Kicinski
In-Reply-To: <20181112225819.29823-1-jakub.kicinski@netronome.com>
Instead of writing the threshold out when Qdisc is configured
and not remembering it move to a scheme where we remember all
thresholds. When configuration changes parse the offloaded
Qdiscs and set thresholds appropriately.
This will help future extensions.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: John Hurley <john.hurley@netronome.com>
---
drivers/net/ethernet/netronome/nfp/abm/ctrl.c | 32 +++----
drivers/net/ethernet/netronome/nfp/abm/main.c | 25 +++++-
drivers/net/ethernet/netronome/nfp/abm/main.h | 16 +++-
.../net/ethernet/netronome/nfp/abm/qdisc.c | 88 ++++++++++++++-----
4 files changed, 120 insertions(+), 41 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/abm/ctrl.c b/drivers/net/ethernet/netronome/nfp/abm/ctrl.c
index 3c661f422688..564427e8a6e8 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/ctrl.c
+++ b/drivers/net/ethernet/netronome/nfp/abm/ctrl.c
@@ -71,35 +71,37 @@ nfp_abm_ctrl_stat_all(struct nfp_abm_link *alink, const struct nfp_rtsym *sym,
return 0;
}
-int nfp_abm_ctrl_set_q_lvl(struct nfp_abm_link *alink, unsigned int i, u32 val)
+int __nfp_abm_ctrl_set_q_lvl(struct nfp_abm *abm, unsigned int id, u32 val)
{
- struct nfp_cpp *cpp = alink->abm->app->cpp;
+ struct nfp_cpp *cpp = abm->app->cpp;
u64 sym_offset;
int err;
- sym_offset = (alink->queue_base + i) * NFP_QLVL_STRIDE + NFP_QLVL_THRS;
- err = __nfp_rtsym_writel(cpp, alink->abm->q_lvls, 4, 0,
- sym_offset, val);
+ __clear_bit(id, abm->threshold_undef);
+ if (abm->thresholds[id] == val)
+ return 0;
+
+ sym_offset = id * NFP_QLVL_STRIDE + NFP_QLVL_THRS;
+ err = __nfp_rtsym_writel(cpp, abm->q_lvls, 4, 0, sym_offset, val);
if (err) {
- nfp_err(cpp, "RED offload setting level failed on vNIC %d queue %d\n",
- alink->id, i);
+ nfp_err(cpp,
+ "RED offload setting level failed on subqueue %d\n",
+ id);
return err;
}
+ abm->thresholds[id] = val;
return 0;
}
-int nfp_abm_ctrl_set_all_q_lvls(struct nfp_abm_link *alink, u32 val)
+int nfp_abm_ctrl_set_q_lvl(struct nfp_abm_link *alink, unsigned int queue,
+ u32 val)
{
- int i, err;
+ unsigned int threshold;
- for (i = 0; i < alink->vnic->max_rx_rings; i++) {
- err = nfp_abm_ctrl_set_q_lvl(alink, i, val);
- if (err)
- return err;
- }
+ threshold = alink->queue_base + queue;
- return 0;
+ return __nfp_abm_ctrl_set_q_lvl(alink->abm, threshold, val);
}
u64 nfp_abm_ctrl_stat_non_sto(struct nfp_abm_link *alink, unsigned int i)
diff --git a/drivers/net/ethernet/netronome/nfp/abm/main.c b/drivers/net/ethernet/netronome/nfp/abm/main.c
index 5e749602989e..c6ae0ac9a154 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/main.c
+++ b/drivers/net/ethernet/netronome/nfp/abm/main.c
@@ -2,6 +2,7 @@
/* Copyright (C) 2018 Netronome Systems, Inc. */
#include <linux/bitfield.h>
+#include <linux/bitmap.h>
#include <linux/etherdevice.h>
#include <linux/lockdep.h>
#include <linux/netdevice.h>
@@ -399,6 +400,7 @@ static int nfp_abm_init(struct nfp_app *app)
struct nfp_pf *pf = app->pf;
struct nfp_reprs *reprs;
struct nfp_abm *abm;
+ unsigned int i;
int err;
if (!pf->eth_tbl) {
@@ -425,15 +427,28 @@ static int nfp_abm_init(struct nfp_app *app)
if (err)
goto err_free_abm;
+ err = -ENOMEM;
+ abm->num_thresholds = NFP_NET_MAX_RX_RINGS;
+ abm->threshold_undef = bitmap_zalloc(abm->num_thresholds, GFP_KERNEL);
+ if (!abm->threshold_undef)
+ goto err_free_abm;
+
+ abm->thresholds = kvcalloc(abm->num_thresholds,
+ sizeof(*abm->thresholds), GFP_KERNEL);
+ if (!abm->thresholds)
+ goto err_free_thresh_umap;
+ for (i = 0; i < NFP_NET_MAX_RX_RINGS; i++)
+ __nfp_abm_ctrl_set_q_lvl(abm, i, NFP_ABM_LVL_INFINITY);
+
/* We start in legacy mode, make sure advanced queuing is disabled */
err = nfp_abm_ctrl_qm_disable(abm);
if (err)
- goto err_free_abm;
+ goto err_free_thresh;
err = -ENOMEM;
reprs = nfp_reprs_alloc(pf->max_data_vnics);
if (!reprs)
- goto err_free_abm;
+ goto err_free_thresh;
RCU_INIT_POINTER(app->reprs[NFP_REPR_TYPE_PHYS_PORT], reprs);
reprs = nfp_reprs_alloc(pf->max_data_vnics);
@@ -445,6 +460,10 @@ static int nfp_abm_init(struct nfp_app *app)
err_free_phys:
nfp_reprs_clean_and_free_by_type(app, NFP_REPR_TYPE_PHYS_PORT);
+err_free_thresh:
+ kvfree(abm->thresholds);
+err_free_thresh_umap:
+ bitmap_free(abm->threshold_undef);
err_free_abm:
kfree(abm);
app->priv = NULL;
@@ -458,6 +477,8 @@ static void nfp_abm_clean(struct nfp_app *app)
nfp_abm_eswitch_clean_up(abm);
nfp_reprs_clean_and_free_by_type(app, NFP_REPR_TYPE_PF);
nfp_reprs_clean_and_free_by_type(app, NFP_REPR_TYPE_PHYS_PORT);
+ bitmap_free(abm->threshold_undef);
+ kvfree(abm->thresholds);
kfree(abm);
app->priv = NULL;
}
diff --git a/drivers/net/ethernet/netronome/nfp/abm/main.h b/drivers/net/ethernet/netronome/nfp/abm/main.h
index a09090004f82..15732ad7c202 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/main.h
+++ b/drivers/net/ethernet/netronome/nfp/abm/main.h
@@ -20,6 +20,11 @@ struct nfp_net;
* struct nfp_abm - ABM NIC app structure
* @app: back pointer to nfp_app
* @pf_id: ID of our PF link
+ *
+ * @thresholds: current threshold configuration
+ * @threshold_undef: bitmap of thresholds which have not been set
+ * @num_thresholds: number of @thresholds and bits in @threshold_undef
+ *
* @eswitch_mode: devlink eswitch mode, advanced functions only visible
* in switchdev mode
* @q_lvls: queue level control area
@@ -28,6 +33,11 @@ struct nfp_net;
struct nfp_abm {
struct nfp_app *app;
unsigned int pf_id;
+
+ u32 *thresholds;
+ unsigned long *threshold_undef;
+ size_t num_thresholds;
+
enum devlink_eswitch_mode eswitch_mode;
const struct nfp_rtsym *q_lvls;
const struct nfp_rtsym *qm_stats;
@@ -64,11 +74,13 @@ struct nfp_alink_xstats {
/**
* struct nfp_red_qdisc - representation of single RED Qdisc
* @handle: handle of currently offloaded RED Qdisc
+ * @threshold: marking threshold of this Qdisc
* @stats: statistics from last refresh
* @xstats: base of extended statistics
*/
struct nfp_red_qdisc {
u32 handle;
+ u32 threshold;
struct nfp_alink_stats stats;
struct nfp_alink_xstats xstats;
};
@@ -102,8 +114,8 @@ int nfp_abm_setup_tc_mq(struct net_device *netdev, struct nfp_abm_link *alink,
void nfp_abm_ctrl_read_params(struct nfp_abm_link *alink);
int nfp_abm_ctrl_find_addrs(struct nfp_abm *abm);
-int nfp_abm_ctrl_set_all_q_lvls(struct nfp_abm_link *alink, u32 val);
-int nfp_abm_ctrl_set_q_lvl(struct nfp_abm_link *alink, unsigned int i,
+int __nfp_abm_ctrl_set_q_lvl(struct nfp_abm *abm, unsigned int id, u32 val);
+int nfp_abm_ctrl_set_q_lvl(struct nfp_abm_link *alink, unsigned int queue,
u32 val);
int nfp_abm_ctrl_read_stats(struct nfp_abm_link *alink,
struct nfp_alink_stats *stats);
diff --git a/drivers/net/ethernet/netronome/nfp/abm/qdisc.c b/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
index abda392880e0..abb0a24c7fac 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
+++ b/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
@@ -7,25 +7,73 @@
#include "../nfpcore/nfp_cpp.h"
#include "../nfp_app.h"
+#include "../nfp_main.h"
+#include "../nfp_net.h"
#include "../nfp_port.h"
#include "main.h"
-static int
+static void
+nfp_abm_offload_compile_red(struct nfp_abm_link *alink,
+ struct nfp_red_qdisc *qdisc, unsigned int queue)
+{
+ if (!qdisc->handle)
+ return;
+
+ nfp_abm_ctrl_set_q_lvl(alink, queue, qdisc->threshold);
+}
+
+static void nfp_abm_offload_compile_one(struct nfp_abm_link *alink)
+{
+ unsigned int i;
+ bool is_mq;
+
+ is_mq = alink->num_qdiscs > 1;
+
+ for (i = 0; i < alink->total_queues; i++) {
+ struct nfp_red_qdisc *next;
+
+ if (is_mq && !alink->red_qdiscs[i].handle)
+ continue;
+
+ next = is_mq ? &alink->red_qdiscs[i] : &alink->red_qdiscs[0];
+ nfp_abm_offload_compile_red(alink, next, i);
+ }
+}
+
+static void nfp_abm_offload_update(struct nfp_abm *abm)
+{
+ struct nfp_abm_link *alink = NULL;
+ struct nfp_pf *pf = abm->app->pf;
+ struct nfp_net *nn;
+ size_t i;
+
+ /* Mark all thresholds as unconfigured */
+ __bitmap_set(abm->threshold_undef, 0, abm->num_thresholds);
+
+ /* Configure all offloads */
+ list_for_each_entry(nn, &pf->vnics, vnic_list) {
+ alink = nn->app_priv;
+ nfp_abm_offload_compile_one(alink);
+ }
+
+ /* Reset the unconfigured thresholds */
+ for (i = 0; i < abm->num_thresholds; i++)
+ if (test_bit(i, abm->threshold_undef))
+ __nfp_abm_ctrl_set_q_lvl(abm, i, NFP_ABM_LVL_INFINITY);
+}
+
+static void
__nfp_abm_reset_root(struct net_device *netdev, struct nfp_abm_link *alink,
u32 handle, unsigned int qs, u32 init_val)
{
struct nfp_port *port = nfp_port_from_netdev(netdev);
- int ret;
- ret = nfp_abm_ctrl_set_all_q_lvls(alink, init_val);
memset(alink->red_qdiscs, 0,
sizeof(*alink->red_qdiscs) * alink->num_qdiscs);
alink->parent = handle;
alink->num_qdiscs = qs;
port->tc_offload_cnt = qs;
-
- return ret;
}
static void
@@ -66,12 +114,12 @@ nfp_abm_red_destroy(struct net_device *netdev, struct nfp_abm_link *alink,
if (i == alink->num_qdiscs)
return;
- if (alink->parent == TC_H_ROOT) {
+ if (alink->parent == TC_H_ROOT)
nfp_abm_reset_root(netdev, alink, TC_H_ROOT, 0);
- } else {
- nfp_abm_ctrl_set_q_lvl(alink, i, NFP_ABM_LVL_INFINITY);
+ else
memset(&alink->red_qdiscs[i], 0, sizeof(*alink->red_qdiscs));
- }
+
+ nfp_abm_offload_update(alink->abm);
}
static bool
@@ -121,29 +169,19 @@ nfp_abm_red_replace(struct net_device *netdev, struct nfp_abm_link *alink,
}
if (existing) {
- if (alink->parent == TC_H_ROOT)
- err = nfp_abm_ctrl_set_all_q_lvls(alink, opt->set.min);
- else
- err = nfp_abm_ctrl_set_q_lvl(alink, i, opt->set.min);
- if (err)
- goto err_destroy;
+ nfp_abm_offload_update(alink->abm);
return 0;
}
if (opt->parent == TC_H_ROOT) {
i = 0;
- err = __nfp_abm_reset_root(netdev, alink, TC_H_ROOT, 1,
- opt->set.min);
+ __nfp_abm_reset_root(netdev, alink, TC_H_ROOT, 1, opt->set.min);
} else if (TC_H_MAJ(alink->parent) == TC_H_MAJ(opt->parent)) {
i = TC_H_MIN(opt->parent) - 1;
- err = nfp_abm_ctrl_set_q_lvl(alink, i, opt->set.min);
} else {
return -EINVAL;
}
- /* Set the handle to try full clean up, in case IO failed */
alink->red_qdiscs[i].handle = opt->handle;
- if (err)
- goto err_destroy;
if (opt->parent == TC_H_ROOT)
err = nfp_abm_ctrl_read_stats(alink,
@@ -163,9 +201,12 @@ nfp_abm_red_replace(struct net_device *netdev, struct nfp_abm_link *alink,
if (err)
goto err_destroy;
+ alink->red_qdiscs[i].threshold = opt->set.min;
alink->red_qdiscs[i].stats.backlog_pkts = 0;
alink->red_qdiscs[i].stats.backlog_bytes = 0;
+ nfp_abm_offload_update(alink->abm);
+
return 0;
err_destroy:
/* If the qdisc keeps on living, but we can't offload undo changes */
@@ -292,10 +333,13 @@ int nfp_abm_setup_tc_mq(struct net_device *netdev, struct nfp_abm_link *alink,
case TC_MQ_CREATE:
nfp_abm_reset_root(netdev, alink, opt->handle,
alink->total_queues);
+ nfp_abm_offload_update(alink->abm);
return 0;
case TC_MQ_DESTROY:
- if (opt->handle == alink->parent)
+ if (opt->handle == alink->parent) {
nfp_abm_reset_root(netdev, alink, TC_H_ROOT, 0);
+ nfp_abm_offload_update(alink->abm);
+ }
return 0;
case TC_MQ_STATS:
return nfp_abm_mq_stats(alink, opt);
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 03/13] nfp: abm: track all offload-enabled qdiscs
From: Jakub Kicinski @ 2018-11-12 22:58 UTC (permalink / raw)
To: davem; +Cc: oss-drivers, netdev, jiri, xiyou.wangcong, jhs, Jakub Kicinski
In-Reply-To: <20181112225819.29823-1-jakub.kicinski@netronome.com>
Allocate an object corresponding to any offloaded qdisc we are
informed about by the kernel. Not only the qdiscs we have a
chance of offloading.
The count of created objects will be used to decide whether
the ethtool TC offload can be disabled, since otherwise we may
miss destroy commands.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: John Hurley <john.hurley@netronome.com>
---
drivers/net/ethernet/netronome/nfp/abm/main.c | 2 +
drivers/net/ethernet/netronome/nfp/abm/main.h | 23 ++++
.../net/ethernet/netronome/nfp/abm/qdisc.c | 111 +++++++++++++++++-
3 files changed, 132 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/abm/main.c b/drivers/net/ethernet/netronome/nfp/abm/main.c
index c6ae0ac9a154..35f3a6054569 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/main.c
+++ b/drivers/net/ethernet/netronome/nfp/abm/main.c
@@ -329,6 +329,7 @@ nfp_abm_vnic_alloc(struct nfp_app *app, struct nfp_net *nn, unsigned int id)
nfp_abm_vnic_set_mac(app->pf, abm, nn, id);
nfp_abm_ctrl_read_params(alink);
+ INIT_RADIX_TREE(&alink->qdiscs, GFP_KERNEL);
return 0;
@@ -344,6 +345,7 @@ static void nfp_abm_vnic_free(struct nfp_app *app, struct nfp_net *nn)
struct nfp_abm_link *alink = nn->app_priv;
nfp_abm_kill_reprs(alink->abm, alink);
+ WARN(!radix_tree_empty(&alink->qdiscs), "left over qdiscs\n");
kvfree(alink->red_qdiscs);
kfree(alink);
}
diff --git a/drivers/net/ethernet/netronome/nfp/abm/main.h b/drivers/net/ethernet/netronome/nfp/abm/main.h
index 15732ad7c202..64cb5ebcf80e 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/main.h
+++ b/drivers/net/ethernet/netronome/nfp/abm/main.h
@@ -5,6 +5,7 @@
#define __NFP_ABM_H__ 1
#include <linux/bits.h>
+#include <linux/radix-tree.h>
#include <net/devlink.h>
#include <net/pkt_cls.h>
@@ -71,6 +72,26 @@ struct nfp_alink_xstats {
u64 pdrop;
};
+enum nfp_qdisc_type {
+ NFP_QDISC_NONE = 0,
+ NFP_QDISC_MQ,
+ NFP_QDISC_RED,
+};
+
+/**
+ * struct nfp_qdisc - tracked TC Qdisc
+ * @netdev: netdev on which Qdisc was created
+ * @type: Qdisc type
+ * @handle: handle of this Qdisc
+ * @parent_handle: handle of the parent (unreliable if Qdisc was grafted)
+ */
+struct nfp_qdisc {
+ struct net_device *netdev;
+ enum nfp_qdisc_type type;
+ u32 handle;
+ u32 parent_handle;
+};
+
/**
* struct nfp_red_qdisc - representation of single RED Qdisc
* @handle: handle of currently offloaded RED Qdisc
@@ -95,6 +116,7 @@ struct nfp_red_qdisc {
* @parent: handle of expected parent, i.e. handle of MQ, or TC_H_ROOT
* @num_qdiscs: number of currently used qdiscs
* @red_qdiscs: array of qdiscs
+ * @qdiscs: all qdiscs recorded by major part of the handle
*/
struct nfp_abm_link {
struct nfp_abm *abm;
@@ -105,6 +127,7 @@ struct nfp_abm_link {
u32 parent;
unsigned int num_qdiscs;
struct nfp_red_qdisc *red_qdiscs;
+ struct radix_tree_root qdiscs;
};
int nfp_abm_setup_tc_red(struct net_device *netdev, struct nfp_abm_link *alink,
diff --git a/drivers/net/ethernet/netronome/nfp/abm/qdisc.c b/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
index abb0a24c7fac..a6f95924656d 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
+++ b/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
@@ -63,17 +63,97 @@ static void nfp_abm_offload_update(struct nfp_abm *abm)
}
static void
-__nfp_abm_reset_root(struct net_device *netdev, struct nfp_abm_link *alink,
- u32 handle, unsigned int qs, u32 init_val)
+nfp_abm_qdisc_free(struct net_device *netdev, struct nfp_abm_link *alink,
+ struct nfp_qdisc *qdisc)
{
struct nfp_port *port = nfp_port_from_netdev(netdev);
+ if (!qdisc)
+ return;
+ WARN_ON(radix_tree_delete(&alink->qdiscs,
+ TC_H_MAJ(qdisc->handle)) != qdisc);
+ kfree(qdisc);
+
+ port->tc_offload_cnt--;
+}
+
+static struct nfp_qdisc *
+nfp_abm_qdisc_alloc(struct net_device *netdev, struct nfp_abm_link *alink,
+ enum nfp_qdisc_type type, u32 parent_handle, u32 handle)
+{
+ struct nfp_port *port = nfp_port_from_netdev(netdev);
+ struct nfp_qdisc *qdisc;
+ int err;
+
+ qdisc = kzalloc(sizeof(*qdisc), GFP_KERNEL);
+ if (!qdisc)
+ return NULL;
+
+ qdisc->netdev = netdev;
+ qdisc->type = type;
+ qdisc->parent_handle = parent_handle;
+ qdisc->handle = handle;
+
+ err = radix_tree_insert(&alink->qdiscs, TC_H_MAJ(qdisc->handle), qdisc);
+ if (err) {
+ nfp_err(alink->abm->app->cpp,
+ "Qdisc insertion into radix tree failed: %d\n", err);
+ goto err_free_qdisc;
+ }
+
+ port->tc_offload_cnt++;
+ return qdisc;
+
+err_free_qdisc:
+ kfree(qdisc);
+ return NULL;
+}
+
+static struct nfp_qdisc *
+nfp_abm_qdisc_find(struct nfp_abm_link *alink, u32 handle)
+{
+ return radix_tree_lookup(&alink->qdiscs, TC_H_MAJ(handle));
+}
+
+static int
+nfp_abm_qdisc_replace(struct net_device *netdev, struct nfp_abm_link *alink,
+ enum nfp_qdisc_type type, u32 parent_handle, u32 handle,
+ struct nfp_qdisc **qdisc)
+{
+ *qdisc = nfp_abm_qdisc_find(alink, handle);
+ if (*qdisc) {
+ if (WARN_ON((*qdisc)->type != type))
+ return -EINVAL;
+ return 0;
+ }
+
+ *qdisc = nfp_abm_qdisc_alloc(netdev, alink, type, parent_handle,
+ handle);
+ return *qdisc ? 0 : -ENOMEM;
+}
+
+static void
+nfp_abm_qdisc_destroy(struct net_device *netdev, struct nfp_abm_link *alink,
+ u32 handle)
+{
+ struct nfp_qdisc *qdisc;
+
+ qdisc = nfp_abm_qdisc_find(alink, handle);
+ if (!qdisc)
+ return;
+
+ nfp_abm_qdisc_free(netdev, alink, qdisc);
+}
+
+static void
+__nfp_abm_reset_root(struct net_device *netdev, struct nfp_abm_link *alink,
+ u32 handle, unsigned int qs, u32 init_val)
+{
memset(alink->red_qdiscs, 0,
sizeof(*alink->red_qdiscs) * alink->num_qdiscs);
alink->parent = handle;
alink->num_qdiscs = qs;
- port->tc_offload_cnt = qs;
}
static void
@@ -108,6 +188,8 @@ nfp_abm_red_destroy(struct net_device *netdev, struct nfp_abm_link *alink,
{
unsigned int i;
+ nfp_abm_qdisc_destroy(netdev, alink, handle);
+
for (i = 0; i < alink->num_qdiscs; i++)
if (handle == alink->red_qdiscs[i].handle)
break;
@@ -157,12 +239,22 @@ static int
nfp_abm_red_replace(struct net_device *netdev, struct nfp_abm_link *alink,
struct tc_red_qopt_offload *opt)
{
+ struct nfp_qdisc *qdisc;
bool existing;
int i, err;
+ int ret;
+
+ ret = nfp_abm_qdisc_replace(netdev, alink, NFP_QDISC_RED, opt->parent,
+ opt->handle, &qdisc);
i = nfp_abm_red_find(alink, opt);
existing = i >= 0;
+ if (ret) {
+ err = ret;
+ goto err_destroy;
+ }
+
if (!nfp_abm_red_check_params(alink, opt)) {
err = -EINVAL;
goto err_destroy;
@@ -326,6 +418,16 @@ nfp_abm_mq_stats(struct nfp_abm_link *alink, struct tc_mq_qopt_offload *opt)
return 0;
}
+static int
+nfp_abm_mq_create(struct net_device *netdev, struct nfp_abm_link *alink,
+ struct tc_mq_qopt_offload *opt)
+{
+ struct nfp_qdisc *qdisc;
+
+ return nfp_abm_qdisc_replace(netdev, alink, NFP_QDISC_MQ,
+ TC_H_ROOT, opt->handle, &qdisc);
+}
+
int nfp_abm_setup_tc_mq(struct net_device *netdev, struct nfp_abm_link *alink,
struct tc_mq_qopt_offload *opt)
{
@@ -334,8 +436,9 @@ int nfp_abm_setup_tc_mq(struct net_device *netdev, struct nfp_abm_link *alink,
nfp_abm_reset_root(netdev, alink, opt->handle,
alink->total_queues);
nfp_abm_offload_update(alink->abm);
- return 0;
+ return nfp_abm_mq_create(netdev, alink, opt);
case TC_MQ_DESTROY:
+ nfp_abm_qdisc_destroy(netdev, alink, opt->handle);
if (opt->handle == alink->parent) {
nfp_abm_reset_root(netdev, alink, TC_H_ROOT, 0);
nfp_abm_offload_update(alink->abm);
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 04/13] net: sched: provide notification for graft on root
From: Jakub Kicinski @ 2018-11-12 22:58 UTC (permalink / raw)
To: davem; +Cc: oss-drivers, netdev, jiri, xiyou.wangcong, jhs, Jakub Kicinski
In-Reply-To: <20181112225819.29823-1-jakub.kicinski@netronome.com>
Drivers are currently not notified when a Qdisc is grafted as root.
This requires special casing Qdiscs added with parent = TC_H_ROOT in
the driver. Also there is no notification sent to the driver when
an existing Qdisc is grafted as root.
Add this very simple notifications, drivers should now be able to
track their Qdisc tree fully.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: John Hurley <john.hurley@netronome.com>
---
include/linux/netdevice.h | 1 +
include/net/pkt_cls.h | 10 ++++++++++
net/sched/sch_api.c | 17 +++++++++++++++++
3 files changed, 28 insertions(+)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 487fa5e0e165..97b4233120e4 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -845,6 +845,7 @@ enum tc_setup_type {
TC_SETUP_QDISC_PRIO,
TC_SETUP_QDISC_MQ,
TC_SETUP_QDISC_ETF,
+ TC_SETUP_ROOT_QDISC,
};
/* These structures hold the attributes of bpf state that are being passed
diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index f6c0cd29dea4..fa31d034231d 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -889,4 +889,14 @@ struct tc_prio_qopt_offload {
};
};
+enum tc_root_command {
+ TC_ROOT_GRAFT,
+};
+
+struct tc_root_qopt_offload {
+ enum tc_root_command command;
+ u32 handle;
+ bool ingress;
+};
+
#endif
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index f55bc50cd0a9..9c88cec7e8a2 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -860,6 +860,21 @@ void qdisc_offload_graft_helper(struct net_device *dev, struct Qdisc *sch,
}
EXPORT_SYMBOL(qdisc_offload_graft_helper);
+static void qdisc_offload_graft_root(struct net_device *dev,
+ struct Qdisc *new, struct Qdisc *old,
+ struct netlink_ext_ack *extack)
+{
+ struct tc_root_qopt_offload graft_offload = {
+ .command = TC_ROOT_GRAFT,
+ .handle = new ? new->handle : 0,
+ .ingress = (new && new->flags & TCQ_F_INGRESS) ||
+ (old && old->flags & TCQ_F_INGRESS),
+ };
+
+ qdisc_offload_graft_helper(dev, NULL, new, old,
+ TC_SETUP_ROOT_QDISC, &graft_offload, extack);
+}
+
static int tc_fill_qdisc(struct sk_buff *skb, struct Qdisc *q, u32 clid,
u32 portid, u32 seq, u16 flags, int event)
{
@@ -1026,6 +1041,8 @@ static int qdisc_graft(struct net_device *dev, struct Qdisc *parent,
if (dev->flags & IFF_UP)
dev_deactivate(dev);
+ qdisc_offload_graft_root(dev, new, old, extack);
+
if (new && new->ops->attach)
goto skip;
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 05/13] nfp: abm: remember which Qdisc is root
From: Jakub Kicinski @ 2018-11-12 22:58 UTC (permalink / raw)
To: davem; +Cc: oss-drivers, netdev, jiri, xiyou.wangcong, jhs, Jakub Kicinski
In-Reply-To: <20181112225819.29823-1-jakub.kicinski@netronome.com>
Keep track of which Qdisc is currently root. We need to implement
TC_SETUP_ROOT_QDISC handling, and for completeness also clear the
root Qdisc pointer when it's freed. TC_SETUP_ROOT_QDISC isn't always
sent when device is dismantled.
Remembering the root Qdisc will allow us to build the entire hierarchy
in following patches.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: John Hurley <john.hurley@netronome.com>
---
drivers/net/ethernet/netronome/nfp/abm/main.c | 2 ++
drivers/net/ethernet/netronome/nfp/abm/main.h | 4 ++++
drivers/net/ethernet/netronome/nfp/abm/qdisc.c | 13 +++++++++++++
3 files changed, 19 insertions(+)
diff --git a/drivers/net/ethernet/netronome/nfp/abm/main.c b/drivers/net/ethernet/netronome/nfp/abm/main.c
index 35f3a6054569..da5394886a78 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/main.c
+++ b/drivers/net/ethernet/netronome/nfp/abm/main.c
@@ -37,6 +37,8 @@ nfp_abm_setup_tc(struct nfp_app *app, struct net_device *netdev,
return -EOPNOTSUPP;
switch (type) {
+ case TC_SETUP_ROOT_QDISC:
+ return nfp_abm_setup_root(netdev, repr->app_priv, type_data);
case TC_SETUP_QDISC_MQ:
return nfp_abm_setup_tc_mq(netdev, repr->app_priv, type_data);
case TC_SETUP_QDISC_RED:
diff --git a/drivers/net/ethernet/netronome/nfp/abm/main.h b/drivers/net/ethernet/netronome/nfp/abm/main.h
index 64cb5ebcf80e..48d519989886 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/main.h
+++ b/drivers/net/ethernet/netronome/nfp/abm/main.h
@@ -116,6 +116,7 @@ struct nfp_red_qdisc {
* @parent: handle of expected parent, i.e. handle of MQ, or TC_H_ROOT
* @num_qdiscs: number of currently used qdiscs
* @red_qdiscs: array of qdiscs
+ * @root_qdisc: pointer to the current root of the Qdisc hierarchy
* @qdiscs: all qdiscs recorded by major part of the handle
*/
struct nfp_abm_link {
@@ -127,9 +128,12 @@ struct nfp_abm_link {
u32 parent;
unsigned int num_qdiscs;
struct nfp_red_qdisc *red_qdiscs;
+ struct nfp_qdisc *root_qdisc;
struct radix_tree_root qdiscs;
};
+int nfp_abm_setup_root(struct net_device *netdev, struct nfp_abm_link *alink,
+ struct tc_root_qopt_offload *opt);
int nfp_abm_setup_tc_red(struct net_device *netdev, struct nfp_abm_link *alink,
struct tc_red_qopt_offload *opt);
int nfp_abm_setup_tc_mq(struct net_device *netdev, struct nfp_abm_link *alink,
diff --git a/drivers/net/ethernet/netronome/nfp/abm/qdisc.c b/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
index a6f95924656d..ba6ce2d1eda2 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
+++ b/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
@@ -143,6 +143,9 @@ nfp_abm_qdisc_destroy(struct net_device *netdev, struct nfp_abm_link *alink,
return;
nfp_abm_qdisc_free(netdev, alink, qdisc);
+
+ if (alink->root_qdisc == qdisc)
+ alink->root_qdisc = NULL;
}
static void
@@ -450,3 +453,13 @@ int nfp_abm_setup_tc_mq(struct net_device *netdev, struct nfp_abm_link *alink,
return -EOPNOTSUPP;
}
}
+
+int nfp_abm_setup_root(struct net_device *netdev, struct nfp_abm_link *alink,
+ struct tc_root_qopt_offload *opt)
+{
+ if (opt->ingress)
+ return -EOPNOTSUPP;
+ alink->root_qdisc = nfp_abm_qdisc_find(alink, opt->handle);
+
+ return 0;
+}
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 06/13] nfp: abm: allocate Qdisc child table
From: Jakub Kicinski @ 2018-11-12 22:58 UTC (permalink / raw)
To: davem; +Cc: oss-drivers, netdev, jiri, xiyou.wangcong, jhs, Jakub Kicinski
In-Reply-To: <20181112225819.29823-1-jakub.kicinski@netronome.com>
To keep track of Qdisc hierarchy allocate a table for children
for each Qdisc. RED Qdisc can only have one child.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: John Hurley <john.hurley@netronome.com>
---
drivers/net/ethernet/netronome/nfp/abm/main.h | 6 +++++
.../net/ethernet/netronome/nfp/abm/qdisc.c | 25 +++++++++++++------
2 files changed, 24 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/abm/main.h b/drivers/net/ethernet/netronome/nfp/abm/main.h
index 48d519989886..adffa36981e0 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/main.h
+++ b/drivers/net/ethernet/netronome/nfp/abm/main.h
@@ -84,12 +84,18 @@ enum nfp_qdisc_type {
* @type: Qdisc type
* @handle: handle of this Qdisc
* @parent_handle: handle of the parent (unreliable if Qdisc was grafted)
+ * @use_cnt: number of attachment points in the hierarchy
+ * @num_children: current size of the @children array
+ * @children: pointers to children
*/
struct nfp_qdisc {
struct net_device *netdev;
enum nfp_qdisc_type type;
u32 handle;
u32 parent_handle;
+ unsigned int use_cnt;
+ unsigned int num_children;
+ struct nfp_qdisc **children;
};
/**
diff --git a/drivers/net/ethernet/netronome/nfp/abm/qdisc.c b/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
index ba6ce2d1eda2..3ecb63060429 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
+++ b/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
@@ -72,6 +72,8 @@ nfp_abm_qdisc_free(struct net_device *netdev, struct nfp_abm_link *alink,
return;
WARN_ON(radix_tree_delete(&alink->qdiscs,
TC_H_MAJ(qdisc->handle)) != qdisc);
+
+ kfree(qdisc->children);
kfree(qdisc);
port->tc_offload_cnt--;
@@ -79,7 +81,8 @@ nfp_abm_qdisc_free(struct net_device *netdev, struct nfp_abm_link *alink,
static struct nfp_qdisc *
nfp_abm_qdisc_alloc(struct net_device *netdev, struct nfp_abm_link *alink,
- enum nfp_qdisc_type type, u32 parent_handle, u32 handle)
+ enum nfp_qdisc_type type, u32 parent_handle, u32 handle,
+ unsigned int children)
{
struct nfp_port *port = nfp_port_from_netdev(netdev);
struct nfp_qdisc *qdisc;
@@ -89,21 +92,28 @@ nfp_abm_qdisc_alloc(struct net_device *netdev, struct nfp_abm_link *alink,
if (!qdisc)
return NULL;
+ qdisc->children = kcalloc(children, sizeof(void *), GFP_KERNEL);
+ if (!qdisc->children)
+ goto err_free_qdisc;
+
qdisc->netdev = netdev;
qdisc->type = type;
qdisc->parent_handle = parent_handle;
qdisc->handle = handle;
+ qdisc->num_children = children;
err = radix_tree_insert(&alink->qdiscs, TC_H_MAJ(qdisc->handle), qdisc);
if (err) {
nfp_err(alink->abm->app->cpp,
"Qdisc insertion into radix tree failed: %d\n", err);
- goto err_free_qdisc;
+ goto err_free_child_tbl;
}
port->tc_offload_cnt++;
return qdisc;
+err_free_child_tbl:
+ kfree(qdisc->children);
err_free_qdisc:
kfree(qdisc);
return NULL;
@@ -118,7 +128,7 @@ nfp_abm_qdisc_find(struct nfp_abm_link *alink, u32 handle)
static int
nfp_abm_qdisc_replace(struct net_device *netdev, struct nfp_abm_link *alink,
enum nfp_qdisc_type type, u32 parent_handle, u32 handle,
- struct nfp_qdisc **qdisc)
+ unsigned int children, struct nfp_qdisc **qdisc)
{
*qdisc = nfp_abm_qdisc_find(alink, handle);
if (*qdisc) {
@@ -127,8 +137,8 @@ nfp_abm_qdisc_replace(struct net_device *netdev, struct nfp_abm_link *alink,
return 0;
}
- *qdisc = nfp_abm_qdisc_alloc(netdev, alink, type, parent_handle,
- handle);
+ *qdisc = nfp_abm_qdisc_alloc(netdev, alink, type, parent_handle, handle,
+ children);
return *qdisc ? 0 : -ENOMEM;
}
@@ -248,7 +258,7 @@ nfp_abm_red_replace(struct net_device *netdev, struct nfp_abm_link *alink,
int ret;
ret = nfp_abm_qdisc_replace(netdev, alink, NFP_QDISC_RED, opt->parent,
- opt->handle, &qdisc);
+ opt->handle, 1, &qdisc);
i = nfp_abm_red_find(alink, opt);
existing = i >= 0;
@@ -428,7 +438,8 @@ nfp_abm_mq_create(struct net_device *netdev, struct nfp_abm_link *alink,
struct nfp_qdisc *qdisc;
return nfp_abm_qdisc_replace(netdev, alink, NFP_QDISC_MQ,
- TC_H_ROOT, opt->handle, &qdisc);
+ TC_H_ROOT, opt->handle,
+ alink->total_queues, &qdisc);
}
int nfp_abm_setup_tc_mq(struct net_device *netdev, struct nfp_abm_link *alink,
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 07/13] net: sched: red: offload a graft notification
From: Jakub Kicinski @ 2018-11-12 22:58 UTC (permalink / raw)
To: davem; +Cc: oss-drivers, netdev, jiri, xiyou.wangcong, jhs, Jakub Kicinski
In-Reply-To: <20181112225819.29823-1-jakub.kicinski@netronome.com>
Drivers offloading Qdiscs should have reasonable certainty
the offloaded behaviour matches the SW path. This is impossible
if the driver does not know about all Qdiscs or when Qdiscs move
and are reused. Send a graft notification from RED. The drivers
are expected to simply stop offloading the Qdisc, if a non-standard
child is ever grafted onto it.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: John Hurley <john.hurley@netronome.com>
---
include/net/pkt_cls.h | 2 ++
net/sched/sch_red.c | 17 +++++++++++++++++
2 files changed, 19 insertions(+)
diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index fa31d034231d..01f2802b7aee 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -834,6 +834,7 @@ enum tc_red_command {
TC_RED_DESTROY,
TC_RED_STATS,
TC_RED_XSTATS,
+ TC_RED_GRAFT,
};
struct tc_red_qopt_offload_params {
@@ -853,6 +854,7 @@ struct tc_red_qopt_offload {
struct tc_red_qopt_offload_params set;
struct tc_qopt_offload_stats stats;
struct red_stats *xstats;
+ u32 child_handle;
};
};
diff --git a/net/sched/sch_red.c b/net/sched/sch_red.c
index a1d08bdd9357..4b5ca172ee2d 100644
--- a/net/sched/sch_red.c
+++ b/net/sched/sch_red.c
@@ -367,6 +367,21 @@ static int red_dump_class(struct Qdisc *sch, unsigned long cl,
return 0;
}
+static void red_graft_offload(struct Qdisc *sch,
+ struct Qdisc *new, struct Qdisc *old,
+ struct netlink_ext_ack *extack)
+{
+ struct tc_red_qopt_offload graft_offload = {
+ .handle = sch->handle,
+ .parent = sch->parent,
+ .child_handle = new->handle,
+ .command = TC_RED_GRAFT,
+ };
+
+ qdisc_offload_graft_helper(qdisc_dev(sch), sch, new, old,
+ TC_SETUP_QDISC_RED, &graft_offload, extack);
+}
+
static int red_graft(struct Qdisc *sch, unsigned long arg, struct Qdisc *new,
struct Qdisc **old, struct netlink_ext_ack *extack)
{
@@ -376,6 +391,8 @@ static int red_graft(struct Qdisc *sch, unsigned long arg, struct Qdisc *new,
new = &noop_qdisc;
*old = qdisc_replace(sch, new, &q->qdisc);
+
+ red_graft_offload(sch, new, *old, extack);
return 0;
}
--
2.17.1
^ permalink raw reply related
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