* Re: [PATCH v5 9/9] task_isolation: kick_all_cpus_sync: don't kick isolated cpus
From: Frederic Weisbecker @ 2020-11-23 22:29 UTC (permalink / raw)
To: Alex Belits
Cc: nitesh@redhat.com, Prasun Kapoor, linux-api@vger.kernel.org,
davem@davemloft.net, trix@redhat.com, mingo@kernel.org,
catalin.marinas@arm.com, rostedt@goodmis.org,
linux-kernel@vger.kernel.org, peterx@redhat.com,
tglx@linutronix.de, linux-arch@vger.kernel.org,
mtosatti@redhat.com, will@kernel.org, peterz@infradead.org,
leon@sidebranch.com, linux-arm-kernel@lists.infradead.org,
pauld@redhat.com, netdev@vger.kernel.org
In-Reply-To: <3236b13f42679031960c5605be20664e90e75223.camel@marvell.com>
On Mon, Nov 23, 2020 at 05:58:42PM +0000, Alex Belits wrote:
> From: Yuri Norov <ynorov@marvell.com>
>
> Make sure that kick_all_cpus_sync() does not call CPUs that are running
> isolated tasks.
>
> Signed-off-by: Yuri Norov <ynorov@marvell.com>
> [abelits@marvell.com: use safe task_isolation_cpumask() implementation]
> Signed-off-by: Alex Belits <abelits@marvell.com>
> ---
> kernel/smp.c | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/smp.c b/kernel/smp.c
> index 4d17501433be..b2faecf58ed0 100644
> --- a/kernel/smp.c
> +++ b/kernel/smp.c
> @@ -932,9 +932,21 @@ static void do_nothing(void *unused)
> */
> void kick_all_cpus_sync(void)
> {
> + struct cpumask mask;
> +
> /* Make sure the change is visible before we kick the cpus */
> smp_mb();
> - smp_call_function(do_nothing, NULL, 1);
> +
> + preempt_disable();
> +#ifdef CONFIG_TASK_ISOLATION
> + cpumask_clear(&mask);
> + task_isolation_cpumask(&mask);
> + cpumask_complement(&mask, &mask);
> +#else
> + cpumask_setall(&mask);
> +#endif
> + smp_call_function_many(&mask, do_nothing, NULL, 1);
> + preempt_enable();
Same comment about IPIs here.
> }
> EXPORT_SYMBOL_GPL(kick_all_cpus_sync);
>
> --
> 2.20.1
>
^ permalink raw reply
* Re: Hardcoded multicast queue length in macvlan.c driver causes poor multicast receive performance
From: Jakub Kicinski @ 2020-11-23 22:30 UTC (permalink / raw)
To: Thomas Karlsson; +Cc: davem@davemloft.net, netdev@vger.kernel.org
In-Reply-To: <147b704ac1d5426fbaa8617289dad648@paneda.se>
On Mon, 23 Nov 2020 14:22:31 +0000 Thomas Karlsson wrote:
> Hello,
>
> There is a special queue handling in macvlan.c for broadcast and
> multicast packages that was arbitrarily set to 1000 in commit
> 07d92d5cc977a7fe1e683e1d4a6f723f7f2778cb . While this is probably
> sufficient for most uses cases it is insufficient to support high
> packet rates. I currently have a setup with 144 000 multicast packets
> incoming per second (144 different live audio RTP streams) and suffer
> very frequent packet loss. With unicast this is not an issue and I
> can in addition to the 144kpps load the macvlan interface with
> another 450mbit/s using iperf.
>
> In order to verify that the queue is the problem I edited the define
> to 100000 and recompiled the kernel module. After replacing it with
> rmmod/insmod I get 0 packet loss (measured over 2 days where I before
> had losses every other second or so) and can also load an additional
> 450 mbit/s multicast traffic using iperf without losses. So basically
> no change in performance between unicast/multicast when it comes to
> lost packets on my machine.
>
> I think It would be best if this queue length was configurable
> somehow. Either an option when creating the macvlan (like how
> bridge/passthrough/etc are set) or at least when loading the module
> (for instance by using a config in /etc/modprobe.d). One size does
> not fit all in this situation.
The former please. You can add a netlink attribute, should be
reasonably straightforward. The other macvlan attrs are defined
under "MACVLAN section" in if_link.h.
^ permalink raw reply
* Re: [PATCH v5 4/9] task_isolation: Add task isolation hooks to arch-independent code
From: Thomas Gleixner @ 2020-11-23 22:31 UTC (permalink / raw)
To: Alex Belits, nitesh@redhat.com, frederic@kernel.org
Cc: Prasun Kapoor, linux-api@vger.kernel.org, davem@davemloft.net,
trix@redhat.com, mingo@kernel.org, catalin.marinas@arm.com,
rostedt@goodmis.org, linux-kernel@vger.kernel.org,
peterx@redhat.com, linux-arch@vger.kernel.org,
mtosatti@redhat.com, will@kernel.org, peterz@infradead.org,
leon@sidebranch.com, linux-arm-kernel@lists.infradead.org,
pauld@redhat.com, netdev@vger.kernel.org
In-Reply-To: <ec4bacce635fed4e77ab46752d41432f270edf83.camel@marvell.com>
Alex,
On Mon, Nov 23 2020 at 17:57, Alex Belits wrote:
> Kernel entry and exit functions for task isolation are added to context
> tracking and common entry points. Common handling of pending work on exit
> to userspace now processes isolation breaking, cleanup and start.
Again: You fail to explain the rationale and just explain what the patch
is doing. I can see what the patch is doing from the patch itself.
> ---
> include/linux/hardirq.h | 2 ++
> include/linux/sched.h | 2 ++
> kernel/context_tracking.c | 5 +++++
> kernel/entry/common.c | 10 +++++++++-
> kernel/irq/irqdesc.c | 5 +++++
At least 3 different subsystems, which means this again failed to be
split into seperate patches.
> extern void synchronize_irq(unsigned int irq);
> @@ -115,6 +116,7 @@ extern void rcu_nmi_exit(void);
> do { \
> lockdep_off(); \
> arch_nmi_enter(); \
> + task_isolation_kernel_enter(); \
Where is the explanation why this is safe and correct vs. this fragile
code path?
> @@ -1762,6 +1763,7 @@ extern char *__get_task_comm(char *to, size_t len, struct task_struct *tsk);
> #ifdef CONFIG_SMP
> static __always_inline void scheduler_ipi(void)
> {
> + task_isolation_kernel_enter();
Why is the scheduler_ipi() special? Just because everything else cannot
happen at all? Oh well...
> #define CREATE_TRACE_POINTS
> #include <trace/events/context_tracking.h>
> @@ -100,6 +101,8 @@ void noinstr __context_tracking_enter(enum ctx_state state)
> __this_cpu_write(context_tracking.state, state);
> }
> context_tracking_recursion_exit();
> +
> + task_isolation_exit_to_user_mode();
Why is this here at all and why is it outside of the recursion
protection
> }
> EXPORT_SYMBOL_GPL(__context_tracking_enter);
>
> @@ -148,6 +151,8 @@ void noinstr __context_tracking_exit(enum ctx_state state)
> if (!context_tracking_recursion_enter())
> return;
>
> + task_isolation_kernel_enter();
while this is inside?
And why has the scheduler_ipi() on x86 call this twice? Just because?
> if (__this_cpu_read(context_tracking.state) == state) {
> if (__this_cpu_read(context_tracking.active)) {
> /*
> diff --git a/kernel/entry/common.c b/kernel/entry/common.c
> static void exit_to_user_mode_prepare(struct pt_regs *regs)
> {
> - unsigned long ti_work = READ_ONCE(current_thread_info()->flags);
> + unsigned long ti_work;
>
> lockdep_assert_irqs_disabled();
>
> + task_isolation_before_pending_work_check();
> +
> + ti_work = READ_ONCE(current_thread_info()->flags);
> +
> if (unlikely(ti_work & EXIT_TO_USER_MODE_WORK))
> ti_work = exit_to_user_mode_loop(regs, ti_work);
>
> + if (unlikely(ti_work & _TIF_TASK_ISOLATION))
> + task_isolation_start();
Where is the explaination of this change?
Aside of that how does anything of this compile on x86 at all?
Answer: It does not ...
Stop this frenzy right now. It's going nowhere and all you achieve is to
make people more grumpy than they are already.
Thanks,
tglx
^ permalink raw reply
* Re: [PATCH net-next] bridge: mrp: Implement LC mode for MRP
From: Horatiu Vultur @ 2020-11-23 22:31 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Nikolay Aleksandrov, roopa, davem, linux-kernel, bridge, netdev
In-Reply-To: <20201123140519.3bb3db16@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
The 11/23/2020 14:05, Jakub Kicinski wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> On Mon, 23 Nov 2020 16:25:53 +0200 Nikolay Aleksandrov wrote:
> > >>> @@ -156,4 +157,10 @@ struct br_mrp_in_link_hdr {
> > >>> __be16 interval;
> > >>> };
> > >>>
> > >>> +struct br_mrp_in_link_status_hdr {
> > >>> + __u8 sa[ETH_ALEN];
> > >>> + __be16 port_role;
> > >>> + __be16 id;
> > >>> +};
> > >>> +
> > >>
> > >> I didn't see this struct used anywhere, am I missing anything?
> > >
> > > Yes, you are right, the struct is not used any. But I put it there as I
> > > put the other frame types for MRP.
> > >
> >
> > I see, we don't usually add unused code. The patch is fine as-is and since
> > this is already the case for other MRP parts I'm not strictly against it, so:
> >
> > Acked-by: Nikolay Aleksandrov <nikolay@nvidia.com>
> >
> > If Jakub decides to adhere to that rule you can keep my acked-by and just remove
> > the struct for v2.
Hi Jakub,
>
> Yes, good catch, let's drop it, we don't want to make too much of
> a precedent for using kernel uAPI headers as a place to provide
> protocol-related structs if the kernel doesn't need them.
OK, I see. I will send a new version for this patch where I will drop
the struct 'br_mrp_in_link_stauts_hdr'.
>
> The existing structs are only present in net-next as well, so if you
> don't mind Horatiu it'd be good to follow up and remove the unused ones
> and move the ones (if any) which are only used by the kernel but not by
> the user space <-> kernel API communication out of include/uapi.
Maybe we don't refer to the same structs, but I could see that they are
already in net and in Linus' tree. For example the struct
'br_mrp_ring_topo_hdr'. Or am I missunderstanding something?
--
/Horatiu
^ permalink raw reply
* Re: [PATCH v5 7/9] task_isolation: don't interrupt CPUs with tick_nohz_full_kick_cpu()
From: Alex Belits @ 2020-11-23 22:35 UTC (permalink / raw)
To: frederic@kernel.org
Cc: Prasun Kapoor, linux-api@vger.kernel.org, davem@davemloft.net,
trix@redhat.com, mingo@kernel.org, linux-kernel@vger.kernel.org,
rostedt@goodmis.org, peterx@redhat.com, tglx@linutronix.de,
nitesh@redhat.com, linux-arch@vger.kernel.org,
mtosatti@redhat.com, will@kernel.org, peterz@infradead.org,
leon@sidebranch.com, linux-arm-kernel@lists.infradead.org,
catalin.marinas@arm.com, pauld@redhat.com, netdev@vger.kernel.org
In-Reply-To: <20201123221336.GB1751@lothringen>
On Mon, 2020-11-23 at 23:13 +0100, Frederic Weisbecker wrote:
> External Email
>
> -------------------------------------------------------------------
> ---
> Hi Alex,
>
> On Mon, Nov 23, 2020 at 05:58:22PM +0000, Alex Belits wrote:
> > From: Yuri Norov <ynorov@marvell.com>
> >
> > For nohz_full CPUs the desirable behavior is to receive interrupts
> > generated by tick_nohz_full_kick_cpu(). But for hard isolation it's
> > obviously not desirable because it breaks isolation.
> >
> > This patch adds check for it.
> >
> > Signed-off-by: Yuri Norov <ynorov@marvell.com>
> > [abelits@marvell.com: updated, only exclude CPUs running isolated
> > tasks]
> > Signed-off-by: Alex Belits <abelits@marvell.com>
> > ---
> > kernel/time/tick-sched.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
> > index a213952541db..6c8679e200f0 100644
> > --- a/kernel/time/tick-sched.c
> > +++ b/kernel/time/tick-sched.c
> > @@ -20,6 +20,7 @@
> > #include <linux/sched/clock.h>
> > #include <linux/sched/stat.h>
> > #include <linux/sched/nohz.h>
> > +#include <linux/isolation.h>
> > #include <linux/module.h>
> > #include <linux/irq_work.h>
> > #include <linux/posix-timers.h>
> > @@ -268,7 +269,8 @@ static void tick_nohz_full_kick(void)
> > */
> > void tick_nohz_full_kick_cpu(int cpu)
> > {
> > - if (!tick_nohz_full_cpu(cpu))
> > + smp_rmb();
> > + if (!tick_nohz_full_cpu(cpu) || task_isolation_on_cpu(cpu))
> > return;
>
> Like I said in subsequent reviews, we are not going to ignore IPIs.
> We must fix the sources of these IPIs instead.
This is what I am working on right now. This is made with an assumption
that CPU running isolated task has no reason to be kicked because
nothing else is supposed to be there. Usually this is true and when not
true is still safe when everything else is behaving right. For this
version I have kept the original implementation with minimal changes to
make it possible to use task isolation at all.
I agree that it's a much better idea is to determine if the CPU should
be kicked. If it really should, that will be a legitimate cause to
break isolation there, because CPU running isolated task has no
legitimate reason to have timers running. Right now I am trying to
determine the origin of timers that _still_ show up as running in the
current kernel version, so I think, this is a rather large chunk of
work that I have to do separately.
--
Alex
^ permalink raw reply
* Re: [PATCH v5 7/9] task_isolation: don't interrupt CPUs with tick_nohz_full_kick_cpu()
From: Thomas Gleixner @ 2020-11-23 22:36 UTC (permalink / raw)
To: Alex Belits, nitesh@redhat.com, frederic@kernel.org
Cc: Prasun Kapoor, linux-api@vger.kernel.org, davem@davemloft.net,
trix@redhat.com, mingo@kernel.org, catalin.marinas@arm.com,
rostedt@goodmis.org, linux-kernel@vger.kernel.org,
peterx@redhat.com, linux-arch@vger.kernel.org,
mtosatti@redhat.com, will@kernel.org, peterz@infradead.org,
leon@sidebranch.com, linux-arm-kernel@lists.infradead.org,
pauld@redhat.com, netdev@vger.kernel.org
In-Reply-To: <76ed0b222d2f16fb5aebd144ac0222a7f3b87fa1.camel@marvell.com>
On Mon, Nov 23 2020 at 17:58, Alex Belits wrote:
> From: Yuri Norov <ynorov@marvell.com>
>
> For nohz_full CPUs the desirable behavior is to receive interrupts
> generated by tick_nohz_full_kick_cpu(). But for hard isolation it's
> obviously not desirable because it breaks isolation.
>
> This patch adds check for it.
git grep 'This patch' Documentation/process/
> */
> void tick_nohz_full_kick_cpu(int cpu)
> {
> - if (!tick_nohz_full_cpu(cpu))
> + smp_rmb();
Undocumented smp_rmb() ...
> + if (!tick_nohz_full_cpu(cpu) || task_isolation_on_cpu(cpu))
> return;
I still have to see a convincing argument why task isolation is special
and not just a straight forward extension of NOHZ full cpu isolation.
It's not special as much as you want it to be special.
Thanks,
tglx
^ permalink raw reply
* Re: [PATCH net-next 00/15] net: phy: add support for shared interrupts (part 3)
From: Jakub Kicinski @ 2020-11-23 22:37 UTC (permalink / raw)
To: Martin Blumenstingl; +Cc: Ioana Ciornei, netdev, linux-kernel
In-Reply-To: <CAFBinCBhWKzQFwERW9cy7T4JdOdFwNOqn2qPqFpqdjbat=-DwA@mail.gmail.com>
On Mon, 23 Nov 2020 23:13:11 +0100 Martin Blumenstingl wrote:
> > Ioana Ciornei (15):
> > net: phy: intel-xway: implement generic .handle_interrupt() callback
> > net: phy: intel-xway: remove the use of .ack_interrupt()
> > net: phy: icplus: implement generic .handle_interrupt() callback
> > net: phy: icplus: remove the use .ack_interrupt()
> > net: phy: meson-gxl: implement generic .handle_interrupt() callback
> > net: phy: meson-gxl: remove the use of .ack_callback()
> I will check the six patches above on Saturday (due to me being very
> busy with my daytime job)
> if that's too late for the netdev maintainers then I'm not worried
> about it. at first glance this looks fine to me. and we can always fix
> things afterwards (but still before -rc1).
That is a little long for patches to be hanging around. I was planning
to apply these on Wed. If either Ioana or you would prefer to get the
testing performed first, please split those patches out and repost once
they get validated.
^ permalink raw reply
* Re: [PATCH net-next v2 1/2] ethtool: Add CMIS 4.0 module type to UAPI
From: Jesse Brandeburg @ 2020-11-23 22:40 UTC (permalink / raw)
To: Moshe Shemesh
Cc: David S. Miller, Jakub Kicinski, Andrew Lunn, Adrian Pop,
Michal Kubecek, netdev, Vladyslav Tarasiuk, Moshe Shemesh
In-Reply-To: <1606123198-6230-2-git-send-email-moshe@mellanox.com>
Moshe Shemesh wrote:
> From: Vladyslav Tarasiuk <vladyslavt@nvidia.com>
>
> CMIS 4.0 document describes a universal EEPROM memory layout, which is
> used for some modules such as DSFP, OSFP and QSFP-DD modules. In order
> to distinguish them in userspace from existing standards, add
> corresponding values.
>
> CMIS 4.0 EERPOM memory includes mandatory and optional pages, the max
typo? s/EERPOM/EEPROM
> read length 768B includes passive and active cables mandatory pages.
>
> Signed-off-by: Vladyslav Tarasiuk <vladyslavt@nvidia.com>
> Reviewed-by: Moshe Shemesh <moshe@nvidia.com>
rest was ok.
^ permalink raw reply
* Re: [EXT] Re: [PATCH v5 9/9] task_isolation: kick_all_cpus_sync: don't kick isolated cpus
From: Alex Belits @ 2020-11-23 22:39 UTC (permalink / raw)
To: frederic@kernel.org
Cc: Prasun Kapoor, linux-api@vger.kernel.org, davem@davemloft.net,
trix@redhat.com, mingo@kernel.org, linux-kernel@vger.kernel.org,
rostedt@goodmis.org, peterx@redhat.com, tglx@linutronix.de,
nitesh@redhat.com, linux-arch@vger.kernel.org,
mtosatti@redhat.com, will@kernel.org, peterz@infradead.org,
leon@sidebranch.com, linux-arm-kernel@lists.infradead.org,
catalin.marinas@arm.com, pauld@redhat.com, netdev@vger.kernel.org
In-Reply-To: <20201123222907.GC1751@lothringen>
On Mon, 2020-11-23 at 23:29 +0100, Frederic Weisbecker wrote:
> External Email
>
> -------------------------------------------------------------------
> ---
> On Mon, Nov 23, 2020 at 05:58:42PM +0000, Alex Belits wrote:
> > From: Yuri Norov <ynorov@marvell.com>
> >
> > Make sure that kick_all_cpus_sync() does not call CPUs that are
> > running
> > isolated tasks.
> >
> > Signed-off-by: Yuri Norov <ynorov@marvell.com>
> > [abelits@marvell.com: use safe task_isolation_cpumask()
> > implementation]
> > Signed-off-by: Alex Belits <abelits@marvell.com>
> > ---
> > kernel/smp.c | 14 +++++++++++++-
> > 1 file changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/kernel/smp.c b/kernel/smp.c
> > index 4d17501433be..b2faecf58ed0 100644
> > --- a/kernel/smp.c
> > +++ b/kernel/smp.c
> > @@ -932,9 +932,21 @@ static void do_nothing(void *unused)
> > */
> > void kick_all_cpus_sync(void)
> > {
> > + struct cpumask mask;
> > +
> > /* Make sure the change is visible before we kick the cpus */
> > smp_mb();
> > - smp_call_function(do_nothing, NULL, 1);
> > +
> > + preempt_disable();
> > +#ifdef CONFIG_TASK_ISOLATION
> > + cpumask_clear(&mask);
> > + task_isolation_cpumask(&mask);
> > + cpumask_complement(&mask, &mask);
> > +#else
> > + cpumask_setall(&mask);
> > +#endif
> > + smp_call_function_many(&mask, do_nothing, NULL, 1);
> > + preempt_enable();
>
> Same comment about IPIs here.
This is different from timers. The original design was based on the
idea that every CPU should be able to enter kernel at any time and run
kernel code with no additional preparation. Then the only solution is
to always do full broadcast and require all CPUs to process it.
What I am trying to introduce is the idea of CPU that is not likely to
run kernel code any soon, and can afford to go through an additional
synchronization procedure on the next entry into kernel. The
synchronization is not skipped, it simply happens later, early in
kernel entry code.
--
Alex
^ permalink raw reply
* Re: [PATCH net-next v2] net: don't include ethtool.h from netdevice.h
From: Jesse Brandeburg @ 2020-11-23 22:41 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, netdev, andrew, mkubecek, linux-rdma, linux-wireless,
Johannes Berg
In-Reply-To: <20201120225052.1427503-1-kuba@kernel.org>
Jakub Kicinski wrote:
> linux/netdevice.h is included in very many places, touching any
> of its dependecies causes large incremental builds.
>
> Drop the linux/ethtool.h include, linux/netdevice.h just needs
> a forward declaration of struct ethtool_ops.
>
> Fix all the places which made use of this implicit include.
>
> Acked-by: Johannes Berg <johannes@sipsolutions.net>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
^ permalink raw reply
* Re: [PATCH net-next] bridge: mrp: Implement LC mode for MRP
From: Jakub Kicinski @ 2020-11-23 22:41 UTC (permalink / raw)
To: Horatiu Vultur
Cc: Nikolay Aleksandrov, roopa, davem, linux-kernel, bridge, netdev
In-Reply-To: <20201123223148.gvexo37ibzophobl@soft-dev3.localdomain>
On Mon, 23 Nov 2020 23:31:48 +0100 Horatiu Vultur wrote:
> > The existing structs are only present in net-next as well, so if you
> > don't mind Horatiu it'd be good to follow up and remove the unused ones
> > and move the ones (if any) which are only used by the kernel but not by
> > the user space <-> kernel API communication out of include/uapi.
>
> Maybe we don't refer to the same structs, but I could see that they are
> already in net and in Linus' tree. For example the struct
> 'br_mrp_ring_topo_hdr'. Or am I missunderstanding something?
Ah, scratch that, I thought this was HSR, I should have paid more
attention. Nothing we can do about the existing ones, then.
^ permalink raw reply
* Re: [PATCH net-next resend 1/2] enetc: Fix endianness issues for enetc_ethtool
From: Jesse Brandeburg @ 2020-11-23 22:42 UTC (permalink / raw)
To: Claudiu Manoil; +Cc: netdev, Jakub Kicinski, David S . Miller
In-Reply-To: <20201119101215.19223-2-claudiu.manoil@nxp.com>
Claudiu Manoil wrote:
> These particular fields are specified in the H/W reference
> manual as having network byte order format, so enforce big
> endian annotation for them and clear the related sparse
> warnings in the process.
>
> Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>
Thanks for fixing these warnings!
Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
^ permalink raw reply
* Re: [PATCH 015/141] netfilter: Fix fall-through warnings for Clang
From: Gustavo A. R. Silva @ 2020-11-23 22:45 UTC (permalink / raw)
To: Florian Westphal
Cc: Pablo Neira Ayuso, Jozsef Kadlecsik, David S. Miller,
Jakub Kicinski, netfilter-devel, coreteam, netdev, linux-kernel,
linux-hardening
In-Reply-To: <20201120224737.GR15137@breakpoint.cc>
On Fri, Nov 20, 2020 at 11:47:37PM +0100, Florian Westphal wrote:
> Gustavo A. R. Silva <gustavoars@kernel.org> wrote:
> > In preparation to enable -Wimplicit-fallthrough for Clang, fix multiple
> > warnings by explicitly adding multiple break statements instead of just
> > letting the code fall through to the next case.
>
> Acked-by: Florian Westphal <fw@strlen.de>
>
> Feel free to carry this in next iteration of series, if any.
Thanks, Florian.
--
Gustavo
^ permalink raw reply
* Re: [EXT] [PATCH 018/141] qed: Fix fall-through warnings for Clang
From: Gustavo A. R. Silva @ 2020-11-23 22:46 UTC (permalink / raw)
To: Igor Russkikh
Cc: Ariel Elior, David S. Miller, Jakub Kicinski, GR-everest-linux-l2,
netdev, linux-kernel, linux-hardening
In-Reply-To: <9bcfa09c-dd8d-f879-4762-1b88779fa397@marvell.com>
On Fri, Nov 20, 2020 at 09:50:06PM +0300, Igor Russkikh wrote:
>
>
> On 20/11/2020 9:26 pm, Gustavo A. R. Silva wrote:
> > External Email
> >
> > ----------------------------------------------------------------------
> > In preparation to enable -Wimplicit-fallthrough for Clang, fix multiple
> > warnings by explicitly adding a couple of break statements instead of
> > just letting the code fall through to the next case.
> >
> > Link:
> > https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_KSPP_linux
> > _issues_115&d=DwIBAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=GtqbaEwqFLiM6BiwNMdKmpXb5o
> > up1VLiSIroUNQwbYA&m=6E7IvGvqcEGj8wEOVoN1BySZhGUVECVTBJCmNiRsHUw&s=J1SWrfEL
> > erJOzUlJdD_S5afGaZosmVP8lyKsu9DTULw&e=
> > Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
>
> Reviewed-by: Igor Russkikh <irusskikh@marvell.com>
Thanks, Igor.
--
Gustavo
^ permalink raw reply
* Re: [PATCH 044/141] net/mlx4: Fix fall-through warnings for Clang
From: Gustavo A. R. Silva @ 2020-11-23 22:49 UTC (permalink / raw)
To: Tariq Toukan
Cc: Tariq Toukan, David S. Miller, Jakub Kicinski, netdev, linux-rdma,
linux-kernel, linux-hardening
In-Reply-To: <0ba92238-2e31-b7d8-5664-72933dc76a7b@gmail.com>
On Sun, Nov 22, 2020 at 10:36:01AM +0200, Tariq Toukan wrote:
>
>
> On 11/20/2020 8:31 PM, Gustavo A. R. Silva wrote:
> > In preparation to enable -Wimplicit-fallthrough for Clang, fix a warning
> > by explicitly adding a break statement instead of just letting the code
> > fall through to the next case.
> >
> > Link: https://github.com/KSPP/linux/issues/115
> > Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> > ---
> > drivers/net/ethernet/mellanox/mlx4/resource_tracker.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c b/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
> > index 1187ef1375e2..e6b8b8dc7894 100644
> > --- a/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
> > +++ b/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
> > @@ -2660,6 +2660,7 @@ int mlx4_FREE_RES_wrapper(struct mlx4_dev *dev, int slave,
> > case RES_XRCD:
> > err = xrcdn_free_res(dev, slave, vhcr->op_modifier, alop,
> > vhcr->in_param, &vhcr->out_param);
> > + break;
> > default:
> > break;
> >
>
> Reviewed-by: Tariq Toukan <tariqt@nvidia.com>
>
> Thanks for your patch.
Thanks, Tariq.
--
Gustavo
^ permalink raw reply
* Re: [PATCH V4 net-next 0/4] net: hns3: updates for -next
From: Jesse Brandeburg @ 2020-11-23 22:55 UTC (permalink / raw)
To: Huazhong Tan
Cc: davem, netdev, linux-kernel, salil.mehta, yisen.zhuang, linuxarm,
kuba
In-Reply-To: <1605514854-11205-1-git-send-email-tanhuazhong@huawei.com>
Huazhong Tan wrote:
> There are several updates relating to the interrupt coalesce for
> the HNS3 ethernet driver.
>
> #1 adds support for QL(quantity limiting, interrupt coalesce
> based on the frame quantity).
> #2 queries the maximum value of GL from the firmware instead of
> a fixed value in code.
> #3 adds support for 1us unit GL(gap limiting, interrupt coalesce
> based on the gap time).
> #4 renames gl_adapt_enable in struct hns3_enet_coalesce to fit
> its new usage.
>
> change log:
> V4 - remove #5~#10 from this series, which needs more discussion.
> V3 - fix a typo error in #1 reported by Jakub Kicinski.
> rewrite #9 commit log.
> remove #11 from this series.
> V2 - reorder #2 & #3 to fix compiler error.
> fix some checkpatch warnings in #10 & #11.
For the series:
Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
^ permalink raw reply
* Re: [PATCH net-next] net: DSCP in IPv4 routing v2
From: Guillaume Nault @ 2020-11-23 22:55 UTC (permalink / raw)
To: Russell Strong; +Cc: netdev
In-Reply-To: <20201121182250.661bfee5@192-168-1-16.tpgi.com.au>
On Sat, Nov 21, 2020 at 06:24:46PM +1000, Russell Strong wrote:
> From 2f27f92d5a6f4dd69ac4af32cdb51ba8d2083606 Mon Sep 17 00:00:00 2001
> From: Russell Strong <russell@strong.id.au>
> Date: Sat, 21 Nov 2020 18:12:43 +1000
> Subject: [PATCH] DSCP in IPv4 routing v2
>
> This patch allows the use of DSCP values in routing
Thanks. There are some problems with this patch though.
About the email:
* Why did you duplicate email headers in the body?
* For the subject, please put the "v2" in the "[PATCH ... ]" part.
* You're modifying many files, but haven't Cc-ed any of their authors
or maintainers.
* The patch content is corrupted.
> Use of TOS macros are replaced with DSCP macros
> where the change does not change the user space API
> with one exception:
>
> net/ipv4/fib_rules.c has been changed to accept a
> wider range of values ( dscp values ). Previously
> this would have returned an error.
Have you really verified that replacing each of these RT_TOS calls had
no unwanted side effect?
RT_TOS didn't clear the second lowest bit, while the new IP_DSCP does.
Therefore, there's no guarantee that such a blanket replacement isn't
going to change existing behaviours. Replacements have to be done
step by step and accompanied by an explanation of why they're safe.
BTW, I think there are some problems with RT_TOS that need to be fixed
separately first.
For example some of the ip6_make_flowinfo() calls can probably
erroneously mark some packets with ECT(0). Instead of masking the
problem in this patch, I think it'd be better to have an explicit fix
that'd mask the ECN bits in ip6_make_flowinfo() and drop the buggy
RT_TOS() in the callers.
Another example is inet_rtm_getroute(). It calls
ip_route_output_key_hash_rcu() without masking the tos field first.
Therefore it can return a different route than what the routing code
would actually use. Like for the ip6_make_flowinfo() case, it might
be better to stop relying on the callers to mask ECN bits and do that
in ip_route_output_key_hash_rcu() instead.
I'll verify that these two problems can actually happen in practice
and will send patches if necessary.
> iproute2 already supports setting dscp values through
> ip route add dsfield <dscp value> lookup ......
>
> Signed-off-by: Russell Strong <russell@strong.id.au>
> ---
> .../ethernet/mellanox/mlx5/core/en/tc_tun.c | 2 +-
> drivers/net/geneve.c | 4 ++--
> drivers/net/ipvlan/ipvlan_core.c | 2 +-
> drivers/net/ppp/pptp.c | 2 +-
> drivers/net/vrf.c | 2 +-
> drivers/net/vxlan.c | 4 ++--
> include/net/ip.h | 2 +-
> include/net/route.h | 6 ++----
> include/uapi/linux/ip.h | 2 ++
> net/bridge/br_netfilter_hooks.c | 2 +-
> net/core/filter.c | 4 ++--
> net/core/lwt_bpf.c | 2 +-
> net/ipv4/fib_frontend.c | 2 +-
> net/ipv4/fib_rules.c | 2 +-
> net/ipv4/icmp.c | 6 +++---
> net/ipv4/ip_gre.c | 2 +-
> net/ipv4/ip_output.c | 2 +-
> net/ipv4/ip_tunnel.c | 6 +++---
> net/ipv4/ipmr.c | 6 +++---
> net/ipv4/netfilter.c | 2 +-
> net/ipv4/netfilter/ipt_rpfilter.c | 2 +-
> net/ipv4/netfilter/nf_dup_ipv4.c | 2 +-
> net/ipv4/route.c | 20 +++++++++----------
> net/ipv6/ip6_output.c | 2 +-
> net/ipv6/ip6_tunnel.c | 4 ++--
> net/ipv6/sit.c | 4 ++--
> net/xfrm/xfrm_policy.c | 2 +-
> 27 files changed, 49 insertions(+), 49 deletions(-)
^ permalink raw reply
* Re: [PATCH net-next 00/15] net: phy: add support for shared interrupts (part 3)
From: Ioana Ciornei @ 2020-11-23 22:55 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: Martin Blumenstingl, Ioana Ciornei, netdev, linux-kernel
In-Reply-To: <20201123143713.6429c056@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
On Mon, Nov 23, 2020 at 02:37:13PM -0800, Jakub Kicinski wrote:
> On Mon, 23 Nov 2020 23:13:11 +0100 Martin Blumenstingl wrote:
> > > Ioana Ciornei (15):
> > > net: phy: intel-xway: implement generic .handle_interrupt() callback
> > > net: phy: intel-xway: remove the use of .ack_interrupt()
> > > net: phy: icplus: implement generic .handle_interrupt() callback
> > > net: phy: icplus: remove the use .ack_interrupt()
> > > net: phy: meson-gxl: implement generic .handle_interrupt() callback
> > > net: phy: meson-gxl: remove the use of .ack_callback()
> > I will check the six patches above on Saturday (due to me being very
> > busy with my daytime job)
> > if that's too late for the netdev maintainers then I'm not worried
> > about it. at first glance this looks fine to me. and we can always fix
> > things afterwards (but still before -rc1).
>
> That is a little long for patches to be hanging around. I was planning
> to apply these on Wed. If either Ioana or you would prefer to get the
> testing performed first, please split those patches out and repost once
> they get validated.
If there is no issue reported in the meantime, I would say to apply the
series. I can always quickly fixup any problems that Martin might find.
Ioana
^ permalink raw reply
* Re: [EXT] Re: [PATCH v5 9/9] task_isolation: kick_all_cpus_sync: don't kick isolated cpus
From: Frederic Weisbecker @ 2020-11-23 23:21 UTC (permalink / raw)
To: Alex Belits
Cc: Prasun Kapoor, linux-api@vger.kernel.org, davem@davemloft.net,
trix@redhat.com, mingo@kernel.org, linux-kernel@vger.kernel.org,
rostedt@goodmis.org, peterx@redhat.com, tglx@linutronix.de,
nitesh@redhat.com, linux-arch@vger.kernel.org,
mtosatti@redhat.com, will@kernel.org, peterz@infradead.org,
leon@sidebranch.com, linux-arm-kernel@lists.infradead.org,
catalin.marinas@arm.com, pauld@redhat.com, netdev@vger.kernel.org
In-Reply-To: <c65ac23c1c408614110635c33eaf4ace98da4343.camel@marvell.com>
On Mon, Nov 23, 2020 at 10:39:34PM +0000, Alex Belits wrote:
>
> On Mon, 2020-11-23 at 23:29 +0100, Frederic Weisbecker wrote:
> > External Email
> >
> > -------------------------------------------------------------------
> > ---
> > On Mon, Nov 23, 2020 at 05:58:42PM +0000, Alex Belits wrote:
> > > From: Yuri Norov <ynorov@marvell.com>
> > >
> > > Make sure that kick_all_cpus_sync() does not call CPUs that are
> > > running
> > > isolated tasks.
> > >
> > > Signed-off-by: Yuri Norov <ynorov@marvell.com>
> > > [abelits@marvell.com: use safe task_isolation_cpumask()
> > > implementation]
> > > Signed-off-by: Alex Belits <abelits@marvell.com>
> > > ---
> > > kernel/smp.c | 14 +++++++++++++-
> > > 1 file changed, 13 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/kernel/smp.c b/kernel/smp.c
> > > index 4d17501433be..b2faecf58ed0 100644
> > > --- a/kernel/smp.c
> > > +++ b/kernel/smp.c
> > > @@ -932,9 +932,21 @@ static void do_nothing(void *unused)
> > > */
> > > void kick_all_cpus_sync(void)
> > > {
> > > + struct cpumask mask;
> > > +
> > > /* Make sure the change is visible before we kick the cpus */
> > > smp_mb();
> > > - smp_call_function(do_nothing, NULL, 1);
> > > +
> > > + preempt_disable();
> > > +#ifdef CONFIG_TASK_ISOLATION
> > > + cpumask_clear(&mask);
> > > + task_isolation_cpumask(&mask);
> > > + cpumask_complement(&mask, &mask);
> > > +#else
> > > + cpumask_setall(&mask);
> > > +#endif
> > > + smp_call_function_many(&mask, do_nothing, NULL, 1);
> > > + preempt_enable();
> >
> > Same comment about IPIs here.
>
> This is different from timers. The original design was based on the
> idea that every CPU should be able to enter kernel at any time and run
> kernel code with no additional preparation. Then the only solution is
> to always do full broadcast and require all CPUs to process it.
>
> What I am trying to introduce is the idea of CPU that is not likely to
> run kernel code any soon, and can afford to go through an additional
> synchronization procedure on the next entry into kernel. The
> synchronization is not skipped, it simply happens later, early in
> kernel entry code.
Ah I see, this is ordered that way:
ll_isol_flags = ISOLATED
CPU 0 CPU 1
------------------ -----------------
// kernel entry
data_to_sync = 1 ll_isol_flags = ISOLATED_BROKEN
smp_mb() smp_mb()
if ll_isol_flags(CPU 1) == ISOLATED READ data_to_sync
smp_call(CPU 1)
You should document that, ie: explain why what you're doing is safe.
Also Beware though that the data to sync in question doesn't need to be visible
in the entry code before task_isolation_kernel_enter(). You need to audit all
the callers of kick_all_cpus_sync().
^ permalink raw reply
* Re: [net-next v3 05/15] ice: create flow profile
From: Jesse Brandeburg @ 2020-11-23 23:21 UTC (permalink / raw)
To: Alexander Duyck
Cc: Nguyen, Anthony L, Cao, Chinh T, davem@davemloft.net,
kuba@kernel.org, Behera, BrijeshX, Valiquette, Real,
sassmann@redhat.com, netdev@vger.kernel.org
In-Reply-To: <CAKgT0Uewo+Rr19EVf9br9zBPsyOUANGMSQ0kqNVAzOJ8cjWMdw@mail.gmail.com>
Alexander Duyck wrote:
> > > I'm not sure this logic is correct. Can the flow director rules
> > > handle
> > > a field that is removed? Last I knew it couldn't. If that is the case
> > > you should be using ACL for any case in which a full mask is not
> > > provided. So in your tests below you could probably drop the check
> > > for
> > > zero as I don't think that is a valid case in which flow director
> > > would work.
> > >
> >
> > I'm not sure what you meant by a field that is removed, but Flow
> > Director can handle reduced input sets. Flow Director is able to handle
> > 0 mask, full mask, and less than 4 tuples. ACL is needed/used only when
> > a partial mask rule is requested.
>
> So historically speaking with flow director you are only allowed one
> mask because it determines the inputs used to generate the hash that
> identifies the flow. So you are only allowed one mask for all flows
> because changing those inputs would break the hash mapping.
>
> Normally this ends up meaning that you have to do like what we did in
> ixgbe and disable ATR and only allow one mask for all inputs. I
> believe for i40e they required that you always use a full 4 tuple. I
> didn't see something like that here. As such you may want to double
> check that you can have a mix of flow director rules that are using 1
> tuple, 2 tuples, 3 tuples, and 4 tuples as last I knew you couldn't.
> Basically if you had fields included they had to be included for all
> the rules on the port or device depending on how the tables are set
> up.
The ice driver hardware is quite a bit more capable than the ixgbe or
i40e hardware, and uses a limited set of ACL rules to support different
sets of masks. We have some limits on the number of masks and the
number of fields that we can simultaneously support, but I think
that is pretty normal for limited hardware resources.
Let's just say that if the code doesn't work on an E810 card then we
messed up and we'll have to fix it. :-)
Thanks for the review! Hope this helps...
^ permalink raw reply
* Re: pull-request: wireless-drivers-2020-11-23
From: Jakub Kicinski @ 2020-11-23 23:30 UTC (permalink / raw)
To: Kalle Valo; +Cc: netdev, linux-wireless
In-Reply-To: <20201123161037.C11D1C43460@smtp.codeaurora.org>
On Mon, 23 Nov 2020 16:10:37 +0000 (UTC) Kalle Valo wrote:
> wireless-drivers fixes for v5.10
>
> First set of fixes for v5.10. One fix for iwlwifi kernel panic, others
> less notable.
>
> rtw88
>
> * fix a bogus test found by clang
>
> iwlwifi
>
> * fix long memory reads causing soft lockup warnings
>
> * fix kernel panic during Channel Switch Announcement (CSA)
>
> * other smaller fixes
>
> MAINTAINERS
>
> * email address updates
Pulled, thanks!
Please watch out for missing sign-offs.
^ permalink raw reply
* Re: [PATCH] lsm,selinux: pass flowi_common instead of flowi to the LSM hooks
From: Paul Moore @ 2020-11-23 23:47 UTC (permalink / raw)
To: James Morris; +Cc: linux-security-module, netdev, selinux, Herbert Xu
In-Reply-To: <alpine.LRH.2.21.2011201401570.20132@namei.org>
On Thu, Nov 19, 2020 at 10:02 PM James Morris <jmorris@namei.org> wrote:
> On Thu, 19 Nov 2020, Paul Moore wrote:
> > As pointed out by Herbert in a recent related patch, the LSM hooks do
> > not have the necessary address family information to use the flowi
> > struct safely. As none of the LSMs currently use any of the protocol
> > specific flowi information, replace the flowi pointers with pointers
> > to the address family independent flowi_common struct.
> >
> > Reported-by: Herbert Xu <herbert@gondor.apana.org.au>
> > Signed-off-by: Paul Moore <paul@paul-moore.com>
>
> Acked-by: James Morris <jamorris@linux.microsoft.com>
Thanks. Seeing no further comments or objections, and given the
discussion in the previous draft of the patch, I've gone again and
merged this into selinux/next.
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: [PATCH bpf-next v3 00/10] Introduce preferred busy-polling
From: Jakub Kicinski @ 2020-11-23 23:54 UTC (permalink / raw)
To: Björn Töpel
Cc: Eric Dumazet, Björn Töpel, Netdev, Karlsson, Magnus,
bpf, Alexei Starovoitov, Daniel Borkmann, Fijalkowski, Maciej,
Samudrala, Sridhar, Brandeburg, Jesse, Zhang, Qi Z,
Jonathan Lemon, maximmi
In-Reply-To: <CAJ+HfNh4Kybjuzi1KOvJBUBvQWFDCZgt7zZNU=ZS8FLCsNKiRQ@mail.gmail.com>
On Mon, 23 Nov 2020 14:31:14 +0100 Björn Töpel wrote:
> Eric/Jakub, any more thoughts/input? Tomatoes? :-P
Looking now, sorry for the delay. Somehow patches without net in their
tag feel like they can wait..
^ permalink raw reply
* [PATCH net v2 0/9] ibmvnic: assorted bug fixes
From: Dany Madden @ 2020-11-23 23:57 UTC (permalink / raw)
To: netdev; +Cc: drt, sukadev, ljp
Assorted fixes for ibmvnic originated from "[PATCH net 00/15] ibmvnic:
assorted bug fixes" sent by Lijun Pan.
V2 Changes as suggested by Jakub Kicinski:
- Added "Fixes" to each patch
- Remove "ibmvnic: process HMC disable command" from the series. Submitting it
separately to net-next.
- Squash V1 "ibmvnic: remove free_all_rwi function" into
ibmvnic: stop free_all_rwi on failed reset
Dany Madden (7):
ibmvnic: handle inconsistent login with reset
ibmvnic: stop free_all_rwi on failed reset
ibmvnic: avoid memset null scrq msgs
ibmvnic: restore adapter state on failed reset
ibmvnic: send_login should check for crq errors
ibmvnic: no reset timeout for 5 seconds after reset
ibmvnic: reduce wait for completion time
Sukadev Bhattiprolu (2):
ibmvnic: delay next reset if hard reset fails
ibmvnic: track pending login
drivers/net/ethernet/ibm/ibmvnic.c | 167 ++++++++++++++++++-----------
drivers/net/ethernet/ibm/ibmvnic.h | 3 +
2 files changed, 105 insertions(+), 65 deletions(-)
--
2.26.2
^ permalink raw reply
* [PATCH net v2 1/9] ibmvnic: handle inconsistent login with reset
From: Dany Madden @ 2020-11-23 23:57 UTC (permalink / raw)
To: netdev; +Cc: drt, sukadev, ljp
In-Reply-To: <20201123235757.6466-1-drt@linux.ibm.com>
Inconsistent login with the vnicserver is causing the device to be
removed. This does not give the device a chance to recover from error
state. This patch schedules a FATAL reset instead to bring the adapter
up.
Fixes: 032c5e82847a2 ("Driver for IBM System i/p VNIC protocol")
Signed-off-by: Dany Madden <drt@linux.ibm.com>
Signed-off-by: Lijun Pan <ljp@linux.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 2aa40b2f225c..dcb23015b6b4 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -4412,7 +4412,7 @@ static int handle_login_rsp(union ibmvnic_crq *login_rsp_crq,
adapter->req_rx_add_queues !=
be32_to_cpu(login_rsp->num_rxadd_subcrqs))) {
dev_err(dev, "FATAL: Inconsistent login and login rsp\n");
- ibmvnic_remove(adapter->vdev);
+ ibmvnic_reset(adapter, VNIC_RESET_FATAL);
return -EIO;
}
size_array = (u64 *)((u8 *)(adapter->login_rsp_buf) +
--
2.26.2
^ 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