* Re: [PATCH net-next v1 1/4] net: phy: broadcom: Add PTP support for some Broadcom PHYs.
From: Jonathan Lemon @ 2022-04-25 23:47 UTC (permalink / raw)
To: Richard Cochran
Cc: f.fainelli, bcm-kernel-feedback-list, andrew, hkallweit1, linux,
netdev, kernel-team
In-Reply-To: <20220425031239.GA6294@hoboy.vegasvil.org>
On Sun, Apr 24, 2022 at 08:12:39PM -0700, Richard Cochran wrote:
> On Sat, Apr 23, 2022 at 07:23:53PM -0700, Jonathan Lemon wrote:
>
> > +static int bcm_ptp_settime_locked(struct bcm_ptp_private *priv,
> > + const struct timespec64 *ts)
> > +{
> > + struct phy_device *phydev = priv->phydev;
> > + u16 ctrl;
> > +
> > + /* set up time code */
> > + bcm_phy_write_exp(phydev, TIME_CODE_0, ts->tv_nsec);
> > + bcm_phy_write_exp(phydev, TIME_CODE_1, ts->tv_nsec >> 16);
> > + bcm_phy_write_exp(phydev, TIME_CODE_2, ts->tv_sec);
> > + bcm_phy_write_exp(phydev, TIME_CODE_3, ts->tv_sec >> 16);
> > + bcm_phy_write_exp(phydev, TIME_CODE_4, ts->tv_sec >> 32);
> > +
> > + /* zero out NCO counter */
> > + bcm_phy_write_exp(phydev, NCO_TIME_0, 0);
> > + bcm_phy_write_exp(phydev, NCO_TIME_1, 0);
> > + bcm_phy_write_exp(phydev, NCO_TIME_2_CTRL, 0);
>
> You are setting the 48 bit counter to zero.
>
> But Lasse's version does this:
>
> // Assign original time codes (48 bit)
> local_time_codes[2] = 0x4000;
> local_time_codes[1] = (u16)(ts->tv_nsec >> 20);
> local_time_codes[0] = (u16)(ts->tv_nsec >> 4);
>
> ...
>
> // Write Local Time Code Register
> bcm_phy_write_exp(phydev, NSE_DPPL_NCO_2_0_REG, local_time_codes[0]);
> bcm_phy_write_exp(phydev, NSE_DPPL_NCO_2_1_REG, local_time_codes[1]);
> bcm_phy_write_exp(phydev, NSE_DPPL_NCO_2_2_REG, local_time_codes[2]);
>
> My understanding is that the PPS output function uses the 48 bit
> counter, and so it ought to be set to a non-zero value.
I'm not sure what this is doing. Setting BIT(14) says this is a
frequency control adjustment. From my understanding, the local timer is
used for generating a oneshot output pulse, which the driver currently
doesn't do.
> In any case, it would be nice to have the 80/48 bit register usage
> clearly explained.
You and me both.
--
Jonathan
^ permalink raw reply
* [PATCH net-next] net: tls: fix async vs NIC crypto offload
From: Jakub Kicinski @ 2022-04-25 23:33 UTC (permalink / raw)
To: davem; +Cc: netdev, pabeni, gal, borisp, john.fastabend, daniel,
Jakub Kicinski
When NIC takes care of crypto (or the record has already
been decrypted) we forget to update darg->async. ->async
is supposed to mean whether record is async capable on
input and whether record has been queued for async crypto
on output.
Reported-by: Gal Pressman <gal@nvidia.com>
Fixes: 3547a1f9d988 ("tls: rx: use async as an in-out argument")
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
net/tls/tls_sw.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index ddbe05ec5489..80094528eadb 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -1562,6 +1562,7 @@ static int decrypt_skb_update(struct sock *sk, struct sk_buff *skb,
if (tlm->decrypted) {
darg->zc = false;
+ darg->async = false;
return 0;
}
@@ -1572,6 +1573,7 @@ static int decrypt_skb_update(struct sock *sk, struct sk_buff *skb,
if (err > 0) {
tlm->decrypted = 1;
darg->zc = false;
+ darg->async = false;
goto decrypt_done;
}
}
--
2.34.1
^ permalink raw reply related
* Re: [PATCH net-next v1 1/4] net: phy: broadcom: Add PTP support for some Broadcom PHYs.
From: Jonathan Lemon @ 2022-04-25 23:30 UTC (permalink / raw)
To: Richard Cochran
Cc: f.fainelli, bcm-kernel-feedback-list, andrew, hkallweit1, linux,
netdev, kernel-team
In-Reply-To: <20220425011931.GB4472@hoboy.vegasvil.org>
On Sun, Apr 24, 2022 at 06:19:31PM -0700, Richard Cochran wrote:
> On Sat, Apr 23, 2022 at 07:23:53PM -0700, Jonathan Lemon wrote:
>
> > +static bool bcm_ptp_rxtstamp(struct mii_timestamper *mii_ts,
> > + struct sk_buff *skb, int type)
> > +{
> > + struct bcm_ptp_private *priv = mii2priv(mii_ts);
> > + struct skb_shared_hwtstamps *hwts;
> > + struct ptp_header *header;
> > + u32 sec, nsec;
> > + u8 *data;
> > +
> > + if (!priv->hwts_rx)
> > + return false;
> > +
> > + header = ptp_parse_header(skb, type);
> > + if (!header)
> > + return false;
> > +
> > + data = (u8 *)(header + 1);
>
> No need to pointer math, as ptp_header already has reserved1 and reserved2.
>
> > + sec = get_unaligned_be32(data);
>
> Something is missing here. The seconds field is only four bits, so
> the code needs to read the 80 bit counter once in a while and augment
> the time stamp with the upper bits.
The BCM chip inserts a 64-bit sec.nsec RX timestamp immediately after
the PTP header. So I'm recovering it here. I'll also update the patch
to memmove() the tail of the skb up in order to remove it, just in case
it makes a difference.
--
Jonathan
^ permalink raw reply
* Re: [PATCH] ath10k: skip ath10k_halt during suspend for driver state RESTARTING
From: Brian Norris @ 2022-04-25 23:13 UTC (permalink / raw)
To: Abhishek Kumar
Cc: kvalo, linux-kernel, linux-wireless, ath10k, netdev, Wen Gong,
David S. Miller, Jakub Kicinski, Paolo Abeni
In-Reply-To: <YmcqsFyCMqcWAEMM@google.com>
On Mon, Apr 25, 2022 at 04:11:44PM -0700, Brian Norris wrote:
> On Mon, Apr 25, 2022 at 02:15:20AM +0000, Abhishek Kumar wrote:
> > --- a/drivers/net/wireless/ath/ath10k/mac.c
> > +++ b/drivers/net/wireless/ath/ath10k/mac.c
> > @@ -5345,8 +5345,22 @@ static void ath10k_stop(struct ieee80211_hw *hw)
...
> > + /* If the current driver state is RESTARTING but not yet
> > + * fully RESTARTED because of incoming suspend event,
> > + * then ath11k_halt is already called via
>
> s/ath11k/ath10k/
>
> I know ath11k is the hot new thing, but this is ath10k ;)
>
> > + * ath10k_core_restart and should not be called here.
>
> s/ath10k/ath11k/
Oh boy, I got *that* backwards! Should be this, obviously:
s/ath11k/ath10k/
> > + */
> > + if (ar->state != ATH10K_STATE_RESTARTING)
> > + ath10k_halt(ar);
> > + else
> > + /* Suspending here, because when in RESTARTING
> > + * state, ath11k_core_stop skips
>
> s/ath10k/ath11k/
Same.
Brian
^ permalink raw reply
* Re: "mm: uninline copy_overflow()" breaks i386 build in Mellanox MLX4
From: Jason Gunthorpe @ 2022-04-25 23:13 UTC (permalink / raw)
To: Mateusz Jończyk
Cc: netdev, linux-kernel@vger.kernel.org, David Laight, Andrew Morton,
Christophe Leroy, Anshuman Khandual, linux-rdma
In-Reply-To: <dbd203b1-3988-4c9c-909c-2d1f7f173a0d@o2.pl>
On Thu, Apr 21, 2022 at 10:47:01PM +0200, Mateusz Jończyk wrote:
> Hello,
>
> commit ad7489d5262d ("mm: uninline copy_overflow()")
>
> breaks for me a build for i386 in the Mellanox MLX4 driver:
>
> In file included from ./arch/x86/include/asm/preempt.h:7,
> from ./include/linux/preempt.h:78,
> from ./include/linux/percpu.h:6,
> from ./include/linux/context_tracking_state.h:5,
> from ./include/linux/hardirq.h:5,
> from drivers/net/ethernet/mellanox/mlx4/cq.c:37:
> In function ‘check_copy_size’,
> inlined from ‘copy_to_user’ at ./include/linux/uaccess.h:159:6,
> inlined from ‘mlx4_init_user_cqes’ at drivers/net/ethernet/mellanox/mlx4/cq.c:317:9,
> inlined from ‘mlx4_cq_alloc’ at drivers/net/ethernet/mellanox/mlx4/cq.c:394:10:
> ./include/linux/thread_info.h:228:4: error: call to ‘__bad_copy_from’ declared with attribute error: copy source size is too small
> 228 | __bad_copy_from();
> | ^~~~~~~~~~~~~~~~~
> make[5]: *** [scripts/Makefile.build:288: drivers/net/ethernet/mellanox/mlx4/cq.o] Błąd 1
> make[4]: *** [scripts/Makefile.build:550: drivers/net/ethernet/mellanox/mlx4] Błąd 2
> make[3]: *** [scripts/Makefile.build:550: drivers/net/ethernet/mellanox] Błąd 2
> make[2]: *** [scripts/Makefile.build:550: drivers/net/ethernet] Błąd 2
> make[1]: *** [scripts/Makefile.build:550: drivers/net] Błąd 2
>
> Reverting this commit fixes the build. Disabling Mellanox Ethernet drivers
> in Kconfig (tested only with also disabling of all Infiniband support) also fixes the build.
>
> It appears that uninlining of copy_overflow() causes GCC to analyze the code deeper.
This looks like a compiler bug to me, array_size(entries, cqe_size)
cannot be known at compile time, so the __builtin_constant_p(bytes)
should be compile time false meaning the other two bad branches should
have been eliminated.
Jason
^ permalink raw reply
* Re: [PATCH] ath10k: skip ath10k_halt during suspend for driver state RESTARTING
From: Brian Norris @ 2022-04-25 23:11 UTC (permalink / raw)
To: Abhishek Kumar
Cc: kvalo, linux-kernel, linux-wireless, ath10k, netdev, Wen Gong,
David S. Miller, Jakub Kicinski, Paolo Abeni
In-Reply-To: <20220425021442.1.I650b809482e1af8d0156ed88b5dc2677a0711d46@changeid>
On Mon, Apr 25, 2022 at 02:15:20AM +0000, Abhishek Kumar wrote:
> --- a/drivers/net/wireless/ath/ath10k/mac.c
> +++ b/drivers/net/wireless/ath/ath10k/mac.c
> @@ -5345,8 +5345,22 @@ static void ath10k_stop(struct ieee80211_hw *hw)
>
> mutex_lock(&ar->conf_mutex);
> if (ar->state != ATH10K_STATE_OFF) {
> - if (!ar->hw_rfkill_on)
> - ath10k_halt(ar);
> + if (!ar->hw_rfkill_on) {
> + /* If the current driver state is RESTARTING but not yet
> + * fully RESTARTED because of incoming suspend event,
> + * then ath11k_halt is already called via
s/ath11k/ath10k/
I know ath11k is the hot new thing, but this is ath10k ;)
> + * ath10k_core_restart and should not be called here.
s/ath10k/ath11k/
> + */
> + if (ar->state != ATH10K_STATE_RESTARTING)
> + ath10k_halt(ar);
> + else
> + /* Suspending here, because when in RESTARTING
> + * state, ath11k_core_stop skips
s/ath10k/ath11k/
> + * ath10k_wait_for_suspend.
> + */
> + ath10k_wait_for_suspend(ar,
> + WMI_PDEV_SUSPEND_AND_DISABLE_INTR);
> + }
> ar->state = ATH10K_STATE_OFF;
> }
> mutex_unlock(&ar->conf_mutex);
Otherwise, I believe this is the right solution to the problem pointed
out in the commit message:
Reviewed-by: Brian Norris <briannorris@chromium.org>
^ permalink raw reply
* Re: [net-next 0/4] devlink: add dry run support for flash update
From: Jacob Keller @ 2022-04-25 23:05 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: Jiri Pirko, netdev@vger.kernel.org
In-Reply-To: <20211008182938.0dea0600@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
On 10/8/2021 6:29 PM, Jakub Kicinski wrote:
> On Sat, 9 Oct 2021 00:32:49 +0000 Keller, Jacob E wrote:
Heh. It is frustrating how easily things slip through the cracks. I
dropped the ball on this.
> I think the current best practice is not to opt-in commands which
> started out as non-strict into strict validation. That said opting
> it in for MAXTYPE validation seems reasonable to me.
>
Opting in would only help future kernels, and would obviously not help
existing kernels which currently do not have strict validation. I
personally would prefer to migrate to strict matching, but understand
the concerns with breaking existing usage.
> Alternatively, as I said, you can just check the max attr for the
> family in user space. CTRL_CMD_GETFAMILY returns it as part of family
> info (CTRL_ATTR_MAXATTR). We can make user space do the rejecting.
>
Yea I think this is the right approach because its the only way for new
userspace to properly protect against using this on an old kernel.
I'll update the iproute2 patches to do that.
Thanks,
Jake
^ permalink raw reply
* Re: [PATCH 1/4] tools/bpf/runqslower: musl compat: explicitly link with libargp if found
From: Dominique Martinet @ 2022-04-25 22:33 UTC (permalink / raw)
To: Daniel Borkmann
Cc: bpf, netdev, linux-kernel, KP Singh, John Fastabend,
Yonghong Song, Song Liu, Martin KaFai Lau, Andrii Nakryiko,
Alexei Starovoitov
In-Reply-To: <80728495-e1fe-21bb-9814-6251648f8359@iogearbox.net>
Daniel Borkmann wrote on Mon, Apr 25, 2022 at 11:35:41PM +0200:
> > I've dropped this patch from my alpine MR[1] and built things directly
> > with make bpftool etc as suggested above, so my suggestion to make it
> > more easily buildable that way is probably the way to go?
> > [1] https://gitlab.alpinelinux.org/alpine/aports/-/merge_requests/33554
>
> Thanks for looking into this, Dominique! I slightly massaged patch 3 & 4
> and applied it to bpf-next tree.
Thanks!
> I don't really mind about patch 1 & 2, though out of tools/bpf/ the only
> one you /really/ might want to package is bpftool. The other tools are on
> the legacy side of things and JIT disasm you can also get via bpftool anyway.
I was thinking the other tools still had their uses, but I'll readily
admit I've never had a need for them so wasn't sure if I should package
them together or not.
I can see the use of bpf_dbg, but it's occasional enough that people who
need it can just build it when they need... Let's drop both patches and
I'll remove the other legacy tools from package as well.
My last concern would then just be to build it more easily. I just
noticed I can actually 'make bpf/bpftool' directly from the tools/
parent directory, but there's no equivalent for _install rules.
Would something like this make sense? (I can resend as proper patch if
so)
----
diff --git a/tools/Makefile b/tools/Makefile
index db2f7b8ebed5..743d242aebb3 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -112,6 +112,9 @@ cpupower_install:
cgroup_install counter_install firewire_install gpio_install hv_install iio_install perf_install bootconfig_install spi_install usb_install virtio_install vm_install bpf_install objtool_install wmi_install pci_install debugging_install tracing_install:
$(call descend,$(@:_install=),install)
+bpf/%_install: FORCE
+ $(call descend,$(@:_install=),install)
+
selftests_install:
$(call descend,testing/$(@:_install=),install)
----
> Given this is not covered by BPF CI, are you planning to regularly check
> for musl compatibility before a new kernel is cut?
alpine doesn't update the 'tools' subpackage with every kernel release,
I'm not sure what the exact schedule is but from the looks of it it
tracks LTS releases with updates every few months within the stable
release or to the next one.
I don't really have any resource to run a regular CI, but I guess I can
check from time to time.. If I ever get around to adding a linux-next
test to work's CI I can check bpftool builds at the same time, but who
knows when that'll ever be.
OTOH I had a first look last year (back when I tried to push
ACTIONRETVAL to musl) and there haven't been any new incompatibility, so
I think it's fine to just deal with minor hiccups when alpine upgrades
once in a while.
--
Dominique
^ permalink raw reply related
* [PATCH 1/2] net: wan: atp: remove unused eeprom_delay()
From: Bjorn Helgaas @ 2022-04-25 21:26 UTC (permalink / raw)
To: Jakub Kicinski, David S . Miller, Paolo Abeni, Chas Williams
Cc: linux-atm-general, netdev, linux-kernel, Bjorn Helgaas
In-Reply-To: <20220425212644.1659070-1-helgaas@kernel.org>
From: Bjorn Helgaas <bhelgaas@google.com>
atp.h is included only by atp.c, which does not use eeprom_delay(). Remove
the unused definition.
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
drivers/net/ethernet/realtek/atp.h | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/net/ethernet/realtek/atp.h b/drivers/net/ethernet/realtek/atp.h
index 63f0d2d0e87b..b202184eddd4 100644
--- a/drivers/net/ethernet/realtek/atp.h
+++ b/drivers/net/ethernet/realtek/atp.h
@@ -255,10 +255,6 @@ static inline void write_word_mode0(short ioaddr, unsigned short value)
#define EE_DATA_WRITE 0x01 /* EEPROM chip data in. */
#define EE_DATA_READ 0x08 /* EEPROM chip data out. */
-/* Delay between EEPROM clock transitions. */
-#define eeprom_delay(ticks) \
-do { int _i = 40; while (--_i > 0) { __SLOW_DOWN_IO; } } while (0)
-
/* The EEPROM commands include the alway-set leading bit. */
#define EE_WRITE_CMD(offset) (((5 << 6) + (offset)) << 17)
#define EE_READ(offset) (((6 << 6) + (offset)) << 17)
--
2.25.1
^ permalink raw reply related
* Re: [PATCH bpf-next] bpf: use bpf_prog_run_array_cg_flags everywhere
From: Stanislav Fomichev @ 2022-04-25 21:48 UTC (permalink / raw)
To: Martin KaFai Lau
Cc: Andrii Nakryiko, Networking, bpf, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko
In-Reply-To: <20220425203759.yxyyvdarx4woegfg@kafai-mbp.dhcp.thefacebook.com>
On Mon, Apr 25, 2022 at 1:38 PM Martin KaFai Lau <kafai@fb.com> wrote:
>
> On Wed, Apr 20, 2022 at 03:30:43PM -0700, Stanislav Fomichev wrote:
> > On Wed, Apr 20, 2022 at 3:04 PM Andrii Nakryiko
> > <andrii.nakryiko@gmail.com> wrote:
> > >
> > > On Tue, Apr 19, 2022 at 3:23 PM Stanislav Fomichev <sdf@google.com> wrote:
> > > >
> > > > Rename bpf_prog_run_array_cg_flags to bpf_prog_run_array_cg and
> > > > use it everywhere. check_return_code already enforces sane
> > > > return ranges for all cgroup types. (only egress and bind hooks have
> > > > uncanonical return ranges, the rest is using [0, 1])
> > > >
> > > > No functional changes.
> > > >
> > > > Suggested-by: Alexei Starovoitov <ast@kernel.org>
> > > > Signed-off-by: Stanislav Fomichev <sdf@google.com>
> > > > ---
> > > > include/linux/bpf-cgroup.h | 8 ++---
> > > > kernel/bpf/cgroup.c | 70 ++++++++++++--------------------------
> > > > 2 files changed, 24 insertions(+), 54 deletions(-)
> > > >
> > > > diff --git a/include/linux/bpf-cgroup.h b/include/linux/bpf-cgroup.h
> > > > index 88a51b242adc..669d96d074ad 100644
> > > > --- a/include/linux/bpf-cgroup.h
> > > > +++ b/include/linux/bpf-cgroup.h
> > > > @@ -225,24 +225,20 @@ static inline bool cgroup_bpf_sock_enabled(struct sock *sk,
> > > >
> > > > #define BPF_CGROUP_RUN_SA_PROG(sk, uaddr, atype) \
> > > > ({ \
> > > > - u32 __unused_flags; \
> > > > int __ret = 0; \
> > > > if (cgroup_bpf_enabled(atype)) \
> > > > __ret = __cgroup_bpf_run_filter_sock_addr(sk, uaddr, atype, \
> > > > - NULL, \
> > > > - &__unused_flags); \
> > > > + NULL, NULL); \
> > > > __ret; \
> > > > })
> > > >
> > > > #define BPF_CGROUP_RUN_SA_PROG_LOCK(sk, uaddr, atype, t_ctx) \
> > > > ({ \
> > > > - u32 __unused_flags; \
> > > > int __ret = 0; \
> > > > if (cgroup_bpf_enabled(atype)) { \
> > > > lock_sock(sk); \
> > > > __ret = __cgroup_bpf_run_filter_sock_addr(sk, uaddr, atype, \
> > > > - t_ctx, \
> > > > - &__unused_flags); \
> > > > + t_ctx, NULL); \
> > > > release_sock(sk); \
> > > > } \
> > > > __ret; \
> > > > diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
> > > > index 0cb6211fcb58..f61eca32c747 100644
> > > > --- a/kernel/bpf/cgroup.c
> > > > +++ b/kernel/bpf/cgroup.c
> > > > @@ -25,50 +25,18 @@ EXPORT_SYMBOL(cgroup_bpf_enabled_key);
> > > > /* __always_inline is necessary to prevent indirect call through run_prog
> > > > * function pointer.
> > > > */
> > > > -static __always_inline int
> > > > -bpf_prog_run_array_cg_flags(const struct cgroup_bpf *cgrp,
> > > > - enum cgroup_bpf_attach_type atype,
> > > > - const void *ctx, bpf_prog_run_fn run_prog,
> > > > - int retval, u32 *ret_flags)
> > > > -{
> > > > - const struct bpf_prog_array_item *item;
> > > > - const struct bpf_prog *prog;
> > > > - const struct bpf_prog_array *array;
> > > > - struct bpf_run_ctx *old_run_ctx;
> > > > - struct bpf_cg_run_ctx run_ctx;
> > > > - u32 func_ret;
> > > > -
> > > > - run_ctx.retval = retval;
> > > > - migrate_disable();
> > > > - rcu_read_lock();
> > > > - array = rcu_dereference(cgrp->effective[atype]);
> > > > - item = &array->items[0];
> > > > - old_run_ctx = bpf_set_run_ctx(&run_ctx.run_ctx);
> > > > - while ((prog = READ_ONCE(item->prog))) {
> > > > - run_ctx.prog_item = item;
> > > > - func_ret = run_prog(prog, ctx);
> > > > - if (!(func_ret & 1) && !IS_ERR_VALUE((long)run_ctx.retval))
> > > > - run_ctx.retval = -EPERM;
> > > > - *(ret_flags) |= (func_ret >> 1);
> > > > - item++;
> > > > - }
> > > > - bpf_reset_run_ctx(old_run_ctx);
> > > > - rcu_read_unlock();
> > > > - migrate_enable();
> > > > - return run_ctx.retval;
> > > > -}
> > > > -
> > > > static __always_inline int
> > > > bpf_prog_run_array_cg(const struct cgroup_bpf *cgrp,
> > > > enum cgroup_bpf_attach_type atype,
> > > > const void *ctx, bpf_prog_run_fn run_prog,
> > > > - int retval)
> > > > + int retval, u32 *ret_flags)
> > > > {
> > > > const struct bpf_prog_array_item *item;
> > > > const struct bpf_prog *prog;
> > > > const struct bpf_prog_array *array;
> > > > struct bpf_run_ctx *old_run_ctx;
> > > > struct bpf_cg_run_ctx run_ctx;
> > > > + u32 func_ret;
> > > >
> > > > run_ctx.retval = retval;
> > > > migrate_disable();
> > > > @@ -78,8 +46,11 @@ bpf_prog_run_array_cg(const struct cgroup_bpf *cgrp,
> > > > old_run_ctx = bpf_set_run_ctx(&run_ctx.run_ctx);
> > > > while ((prog = READ_ONCE(item->prog))) {
> > > > run_ctx.prog_item = item;
> > > > - if (!run_prog(prog, ctx) && !IS_ERR_VALUE((long)run_ctx.retval))
> > > > + func_ret = run_prog(prog, ctx);
> > > > + if (!(func_ret & 1) && !IS_ERR_VALUE((long)run_ctx.retval))
> > >
> > > to be completely true to previous behavior, shouldn't there be
> > >
> > > if (ret_flags)
> > > func_ret &= 1;
> > > if (!func_ret && !IS_ERR_VALUE(...))
> > >
> > > here?
> > >
> > > This might have been discussed previously and I missed it. If that's
> > > so, please ignore.
> >
> > We are converting the cases where run_prog(prog, ctx) returns 0 or 1,
> > so it seems like we don't have to reproduce the existing behavior
> > 1-to-1?
> > So I'm not sure it matters, or am I missing something?
> A nit, how about testing 'if (ret_flags)' first such that
> it is obvious which case will use higher bits in the return value.
> The compiler may be able to optimize the ret_flags == NULL case also ?
>
> Something like:
>
> func_ret = run_prog(prog, ctx);
> /* The cg bpf prog uses the higher bits of the return value */
> if (ret_flags) {
> *(ret_flags) |= (func_ret >> 1);
> func_ret &= 1;
> }
> if (!func_ret && !IS_ERR_VALUE((long)run_ctx.retval))
> run_ctx.retval = -EPERM;
Sure, this should also address Andrii's point I think. Will resend a v2.
^ permalink raw reply
* [PATCH net-next] net: dsa: mt753x: fix pcs conversion regression
From: Russell King (Oracle) @ 2022-04-25 21:28 UTC (permalink / raw)
To: Landen Chao, DENG Qingfang, Sean Wang
Cc: Daniel Golle, Andrew Lunn, Vivien Didelot, Florian Fainelli,
Vladimir Oltean, David S. Miller, Jakub Kicinski, Paolo Abeni,
Matthias Brugger, netdev, linux-arm-kernel, linux-mediatek
Daniel Golle reports that the conversion of mt753x to phylink PCS caused
an oops as below.
The problem is with the placement of the PCS initialisation, which
occurs after mt7531_setup() has been called. However, burited in this
function is a call to setup the CPU port, which requires the PCS
structure to be already setup.
Fix this by changing the initialisation order.
Unable to handle kernel NULL pointer dereference at virtual address 0000000000000020
Mem abort info:
ESR = 0x96000005
EC = 0x25: DABT (current EL), IL = 32 bits
SET = 0, FnV = 0
EA = 0, S1PTW = 0
FSC = 0x05: level 1 translation fault
Data abort info:
ISV = 0, ISS = 0x00000005
CM = 0, WnR = 0
user pgtable: 4k pages, 39-bit VAs, pgdp=0000000046057000
[0000000000000020] pgd=0000000000000000, p4d=0000000000000000, pud=0000000000000000
Internal error: Oops: 96000005 [#1] SMP
Modules linked in:
CPU: 0 PID: 32 Comm: kworker/u4:1 Tainted: G S 5.18.0-rc3-next-20220422+ #0
Hardware name: Bananapi BPI-R64 (DT)
Workqueue: events_unbound deferred_probe_work_func
pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : mt7531_cpu_port_config+0xcc/0x1b0
lr : mt7531_cpu_port_config+0xc0/0x1b0
sp : ffffffc008d5b980
x29: ffffffc008d5b990 x28: ffffff80060562c8 x27: 00000000f805633b
x26: ffffff80001a8880 x25: 00000000000009c4 x24: 0000000000000016
x23: ffffff8005eb6470 x22: 0000000000003600 x21: ffffff8006948080
x20: 0000000000000000 x19: 0000000000000006 x18: 0000000000000000
x17: 0000000000000001 x16: 0000000000000001 x15: 02963607fcee069e
x14: 0000000000000000 x13: 0000000000000030 x12: 0101010101010101
x11: ffffffc037302000 x10: 0000000000000870 x9 : ffffffc008d5b800
x8 : ffffff800028f950 x7 : 0000000000000001 x6 : 00000000662b3000
x5 : 00000000000002f0 x4 : 0000000000000000 x3 : ffffff800028f080
x2 : 0000000000000000 x1 : ffffff800028f080 x0 : 0000000000000000
Call trace:
mt7531_cpu_port_config+0xcc/0x1b0
mt753x_cpu_port_enable+0x24/0x1f0
mt7531_setup+0x49c/0x5c0
mt753x_setup+0x20/0x31c
dsa_register_switch+0x8bc/0x1020
mt7530_probe+0x118/0x200
mdio_probe+0x30/0x64
really_probe.part.0+0x98/0x280
__driver_probe_device+0x94/0x140
driver_probe_device+0x40/0x114
__device_attach_driver+0xb0/0x10c
bus_for_each_drv+0x64/0xa0
__device_attach+0xa8/0x16c
device_initial_probe+0x10/0x20
bus_probe_device+0x94/0x9c
deferred_probe_work_func+0x80/0xb4
process_one_work+0x200/0x3a0
worker_thread+0x260/0x4c0
kthread+0xd4/0xe0
ret_from_fork+0x10/0x20
Code: 9409e911 937b7e60 8b0002a0 f9405800 (f9401005)
---[ end trace 0000000000000000 ]---
Reported-by: Daniel Golle <daniel@makrotopia.org>
Tested-by: Daniel Golle <daniel@makrotopia.org>
Fixes: cbd1f243bc41 ("net: dsa: mt7530: partially convert to phylink_pcs")
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
Fixes line added so people know which net-next commit is affected;
this is only in net-next at present. If you think it's unnecessary,
please remove when applying the patch, or let me know and I will
resend.
Thanks.
drivers/net/dsa/mt7530.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index c4ea73d996e8..e55d962fef9f 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -3035,9 +3035,16 @@ static int
mt753x_setup(struct dsa_switch *ds)
{
struct mt7530_priv *priv = ds->priv;
- int ret = priv->info->sw_setup(ds);
- int i;
+ int i, ret;
+
+ /* Initialise the PCS devices */
+ for (i = 0; i < priv->ds->num_ports; i++) {
+ priv->pcs[i].pcs.ops = priv->info->pcs_ops;
+ priv->pcs[i].priv = priv;
+ priv->pcs[i].port = i;
+ }
+ ret = priv->info->sw_setup(ds);
if (ret)
return ret;
@@ -3049,13 +3056,6 @@ mt753x_setup(struct dsa_switch *ds)
if (ret && priv->irq)
mt7530_free_irq_common(priv);
- /* Initialise the PCS devices */
- for (i = 0; i < priv->ds->num_ports; i++) {
- priv->pcs[i].pcs.ops = priv->info->pcs_ops;
- priv->pcs[i].priv = priv;
- priv->pcs[i].port = i;
- }
-
return ret;
}
--
2.30.2
^ permalink raw reply related
* Re: [PATCH 4/4] tools/bpf: replace sys/fcntl.h by fcntl.h
From: Quentin Monnet @ 2022-04-25 21:25 UTC (permalink / raw)
To: Dominique Martinet
Cc: bpf, Networking, open list, KP Singh, John Fastabend,
Yonghong Song, Song Liu, Martin KaFai Lau, Andrii Nakryiko,
Daniel Borkmann, Alexei Starovoitov
In-Reply-To: <20220424051022.2619648-5-asmadeus@codewreck.org>
On Sun, 24 Apr 2022 at 06:11, Dominique Martinet <asmadeus@codewreck.org> wrote:
>
> musl does not like including sys/fcntl.h directly:
> 1 | #warning redirecting incorrect #include <sys/fcntl.h> to <fcntl.h>
>
> Signed-off-by: Dominique Martinet <asmadeus@codewreck.org>
Acked-by: Quentin Monnet <quentin@isovalent.com>
^ permalink raw reply
* Re: [PATCH 3/4] tools/bpf: musl compat: replace nftw with FTW_ACTIONRETVAL
From: Quentin Monnet @ 2022-04-25 21:24 UTC (permalink / raw)
To: Dominique Martinet
Cc: bpf, Networking, open list, KP Singh, John Fastabend,
Yonghong Song, Song Liu, Martin KaFai Lau, Andrii Nakryiko,
Daniel Borkmann, Alexei Starovoitov
In-Reply-To: <20220424051022.2619648-4-asmadeus@codewreck.org>
On Sun, 24 Apr 2022 at 06:11, Dominique Martinet <asmadeus@codewreck.org> wrote:
>
> musl nftw implementation does not support FTW_ACTIONRETVAL.
>
> There have been multiple attempts at pushing the feature in musl
> upstream but it has been refused or ignored all the times:
> https://www.openwall.com/lists/musl/2021/03/26/1
> https://www.openwall.com/lists/musl/2022/01/22/1
>
> In this case we only care about /proc/<pid>/fd/<fd>, so it's not
> too difficult to reimplement directly instead, and the new
> implementation makes 'bpftool perf' slightly faster because it doesn't
> needlessly stat/readdir unneeded directories (54ms -> 13ms on my machine)
>
> Signed-off-by: Dominique Martinet <asmadeus@codewreck.org>
Acked-by: Quentin Monnet <quentin@isovalent.com>
^ permalink raw reply
* [PATCH bpf-next v2] bpf: use bpf_prog_run_array_cg_flags everywhere
From: Stanislav Fomichev @ 2022-04-25 22:04 UTC (permalink / raw)
To: netdev, bpf; +Cc: ast, daniel, andrii, Stanislav Fomichev, Martin KaFai Lau
Rename bpf_prog_run_array_cg_flags to bpf_prog_run_array_cg and
use it everywhere. check_return_code already enforces sane
return ranges for all cgroup types. (only egress and bind hooks have
uncanonical return ranges, the rest is using [0, 1])
No functional changes.
v2:
- 'func_ret & 1' under explicit test (Andrii & Martin)
Cc: Martin KaFai Lau <kafai@fb.com>
Suggested-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
include/linux/bpf-cgroup.h | 8 ++---
kernel/bpf/cgroup.c | 72 +++++++++++++-------------------------
2 files changed, 26 insertions(+), 54 deletions(-)
diff --git a/include/linux/bpf-cgroup.h b/include/linux/bpf-cgroup.h
index 88a51b242adc..669d96d074ad 100644
--- a/include/linux/bpf-cgroup.h
+++ b/include/linux/bpf-cgroup.h
@@ -225,24 +225,20 @@ static inline bool cgroup_bpf_sock_enabled(struct sock *sk,
#define BPF_CGROUP_RUN_SA_PROG(sk, uaddr, atype) \
({ \
- u32 __unused_flags; \
int __ret = 0; \
if (cgroup_bpf_enabled(atype)) \
__ret = __cgroup_bpf_run_filter_sock_addr(sk, uaddr, atype, \
- NULL, \
- &__unused_flags); \
+ NULL, NULL); \
__ret; \
})
#define BPF_CGROUP_RUN_SA_PROG_LOCK(sk, uaddr, atype, t_ctx) \
({ \
- u32 __unused_flags; \
int __ret = 0; \
if (cgroup_bpf_enabled(atype)) { \
lock_sock(sk); \
__ret = __cgroup_bpf_run_filter_sock_addr(sk, uaddr, atype, \
- t_ctx, \
- &__unused_flags); \
+ t_ctx, NULL); \
release_sock(sk); \
} \
__ret; \
diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index 0cb6211fcb58..afb414b26d01 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -25,50 +25,18 @@ EXPORT_SYMBOL(cgroup_bpf_enabled_key);
/* __always_inline is necessary to prevent indirect call through run_prog
* function pointer.
*/
-static __always_inline int
-bpf_prog_run_array_cg_flags(const struct cgroup_bpf *cgrp,
- enum cgroup_bpf_attach_type atype,
- const void *ctx, bpf_prog_run_fn run_prog,
- int retval, u32 *ret_flags)
-{
- const struct bpf_prog_array_item *item;
- const struct bpf_prog *prog;
- const struct bpf_prog_array *array;
- struct bpf_run_ctx *old_run_ctx;
- struct bpf_cg_run_ctx run_ctx;
- u32 func_ret;
-
- run_ctx.retval = retval;
- migrate_disable();
- rcu_read_lock();
- array = rcu_dereference(cgrp->effective[atype]);
- item = &array->items[0];
- old_run_ctx = bpf_set_run_ctx(&run_ctx.run_ctx);
- while ((prog = READ_ONCE(item->prog))) {
- run_ctx.prog_item = item;
- func_ret = run_prog(prog, ctx);
- if (!(func_ret & 1) && !IS_ERR_VALUE((long)run_ctx.retval))
- run_ctx.retval = -EPERM;
- *(ret_flags) |= (func_ret >> 1);
- item++;
- }
- bpf_reset_run_ctx(old_run_ctx);
- rcu_read_unlock();
- migrate_enable();
- return run_ctx.retval;
-}
-
static __always_inline int
bpf_prog_run_array_cg(const struct cgroup_bpf *cgrp,
enum cgroup_bpf_attach_type atype,
const void *ctx, bpf_prog_run_fn run_prog,
- int retval)
+ int retval, u32 *ret_flags)
{
const struct bpf_prog_array_item *item;
const struct bpf_prog *prog;
const struct bpf_prog_array *array;
struct bpf_run_ctx *old_run_ctx;
struct bpf_cg_run_ctx run_ctx;
+ u32 func_ret;
run_ctx.retval = retval;
migrate_disable();
@@ -78,7 +46,12 @@ bpf_prog_run_array_cg(const struct cgroup_bpf *cgrp,
old_run_ctx = bpf_set_run_ctx(&run_ctx.run_ctx);
while ((prog = READ_ONCE(item->prog))) {
run_ctx.prog_item = item;
- if (!run_prog(prog, ctx) && !IS_ERR_VALUE((long)run_ctx.retval))
+ func_ret = run_prog(prog, ctx);
+ if (ret_flags) {
+ *(ret_flags) |= (func_ret >> 1);
+ func_ret &= 1;
+ }
+ if (!func_ret && !IS_ERR_VALUE((long)run_ctx.retval))
run_ctx.retval = -EPERM;
item++;
}
@@ -1144,9 +1117,8 @@ int __cgroup_bpf_run_filter_skb(struct sock *sk,
u32 flags = 0;
bool cn;
- ret = bpf_prog_run_array_cg_flags(
- &cgrp->bpf, atype,
- skb, __bpf_prog_run_save_cb, 0, &flags);
+ ret = bpf_prog_run_array_cg(&cgrp->bpf, atype, skb,
+ __bpf_prog_run_save_cb, 0, &flags);
/* Return values of CGROUP EGRESS BPF programs are:
* 0: drop packet
@@ -1172,7 +1144,8 @@ int __cgroup_bpf_run_filter_skb(struct sock *sk,
ret = (cn ? NET_XMIT_DROP : ret);
} else {
ret = bpf_prog_run_array_cg(&cgrp->bpf, atype,
- skb, __bpf_prog_run_save_cb, 0);
+ skb, __bpf_prog_run_save_cb, 0,
+ NULL);
if (ret && !IS_ERR_VALUE((long)ret))
ret = -EFAULT;
}
@@ -1202,7 +1175,8 @@ int __cgroup_bpf_run_filter_sk(struct sock *sk,
{
struct cgroup *cgrp = sock_cgroup_ptr(&sk->sk_cgrp_data);
- return bpf_prog_run_array_cg(&cgrp->bpf, atype, sk, bpf_prog_run, 0);
+ return bpf_prog_run_array_cg(&cgrp->bpf, atype, sk, bpf_prog_run, 0,
+ NULL);
}
EXPORT_SYMBOL(__cgroup_bpf_run_filter_sk);
@@ -1247,8 +1221,8 @@ int __cgroup_bpf_run_filter_sock_addr(struct sock *sk,
}
cgrp = sock_cgroup_ptr(&sk->sk_cgrp_data);
- return bpf_prog_run_array_cg_flags(&cgrp->bpf, atype,
- &ctx, bpf_prog_run, 0, flags);
+ return bpf_prog_run_array_cg(&cgrp->bpf, atype, &ctx, bpf_prog_run,
+ 0, flags);
}
EXPORT_SYMBOL(__cgroup_bpf_run_filter_sock_addr);
@@ -1275,7 +1249,7 @@ int __cgroup_bpf_run_filter_sock_ops(struct sock *sk,
struct cgroup *cgrp = sock_cgroup_ptr(&sk->sk_cgrp_data);
return bpf_prog_run_array_cg(&cgrp->bpf, atype, sock_ops, bpf_prog_run,
- 0);
+ 0, NULL);
}
EXPORT_SYMBOL(__cgroup_bpf_run_filter_sock_ops);
@@ -1292,7 +1266,8 @@ int __cgroup_bpf_check_dev_permission(short dev_type, u32 major, u32 minor,
rcu_read_lock();
cgrp = task_dfl_cgroup(current);
- ret = bpf_prog_run_array_cg(&cgrp->bpf, atype, &ctx, bpf_prog_run, 0);
+ ret = bpf_prog_run_array_cg(&cgrp->bpf, atype, &ctx, bpf_prog_run, 0,
+ NULL);
rcu_read_unlock();
return ret;
@@ -1457,7 +1432,8 @@ int __cgroup_bpf_run_filter_sysctl(struct ctl_table_header *head,
rcu_read_lock();
cgrp = task_dfl_cgroup(current);
- ret = bpf_prog_run_array_cg(&cgrp->bpf, atype, &ctx, bpf_prog_run, 0);
+ ret = bpf_prog_run_array_cg(&cgrp->bpf, atype, &ctx, bpf_prog_run, 0,
+ NULL);
rcu_read_unlock();
kfree(ctx.cur_val);
@@ -1550,7 +1526,7 @@ int __cgroup_bpf_run_filter_setsockopt(struct sock *sk, int *level,
lock_sock(sk);
ret = bpf_prog_run_array_cg(&cgrp->bpf, CGROUP_SETSOCKOPT,
- &ctx, bpf_prog_run, 0);
+ &ctx, bpf_prog_run, 0, NULL);
release_sock(sk);
if (ret)
@@ -1650,7 +1626,7 @@ int __cgroup_bpf_run_filter_getsockopt(struct sock *sk, int level,
lock_sock(sk);
ret = bpf_prog_run_array_cg(&cgrp->bpf, CGROUP_GETSOCKOPT,
- &ctx, bpf_prog_run, retval);
+ &ctx, bpf_prog_run, retval, NULL);
release_sock(sk);
if (ret < 0)
@@ -1699,7 +1675,7 @@ int __cgroup_bpf_run_filter_getsockopt_kern(struct sock *sk, int level,
*/
ret = bpf_prog_run_array_cg(&cgrp->bpf, CGROUP_GETSOCKOPT,
- &ctx, bpf_prog_run, retval);
+ &ctx, bpf_prog_run, retval, NULL);
if (ret < 0)
return ret;
--
2.36.0.rc2.479.g8af0fa9b8e-goog
^ permalink raw reply related
* [PATCH 0/2] net: Remove unused __SLOW_DOWN_IO
From: Bjorn Helgaas @ 2022-04-25 21:26 UTC (permalink / raw)
To: Jakub Kicinski, David S . Miller, Paolo Abeni, Chas Williams
Cc: linux-atm-general, netdev, linux-kernel, Bjorn Helgaas
From: Bjorn Helgaas <bhelgaas@google.com>
Remove unused mentions of __SLOW_DOWN_IO.
Bjorn Helgaas (2):
net: wan: atp: remove unused eeprom_delay()
net: remove comments that mention obsolete __SLOW_DOWN_IO
drivers/atm/nicstarmac.c | 5 -----
drivers/net/ethernet/dec/tulip/winbond-840.c | 2 --
drivers/net/ethernet/natsemi/natsemi.c | 2 --
drivers/net/ethernet/realtek/atp.h | 4 ----
4 files changed, 13 deletions(-)
--
2.25.1
^ permalink raw reply
* Re: [PATCH] net: linkwatch: ignore events for unregistered netdevs
From: Eric Dumazet @ 2022-04-25 21:39 UTC (permalink / raw)
To: Lukas Wunner
Cc: Jakub Kicinski, Paolo Abeni, Oliver Neukum, David S. Miller,
Jann Horn, Oleksij Rempel, netdev, USB list, Andrew Lunn,
Jacky Chou, Willy Tarreau, Lino Sanfilippo, Philipp Rosenberger,
Heiner Kallweit, Greg Kroah-Hartman
In-Reply-To: <20220425211822.GA22629@wunner.de>
On Mon, Apr 25, 2022 at 2:18 PM Lukas Wunner <lukas@wunner.de> wrote:
>
> On Mon, Apr 25, 2022 at 07:41:46AM -0700, Jakub Kicinski wrote:
> > On Sat, 23 Apr 2022 18:07:23 +0200 Lukas Wunner wrote:
> > > > Looking at the original report it looks like the issue could be
> > > > resolved with a more usb-specific change: e.g. it looks like
> > > > usbnet_defer_kevent() is not acquiring a dev reference as it should.
> > > >
> > > > Have you considered that path?
> > >
> > > First of all, the diffstat of the patch shows this is an opportunity
> > > to reduce LoC as well as simplify and speed up device teardown.
> > >
> > > Second, the approach you're proposing won't work if a driver calls
> > > netif_carrier_on/off() after unregister_netdev().
> > >
> > > It seems prudent to prevent such a misbehavior in *any* driver,
> > > not just usbnet. usbnet may not be the only one doing it wrong.
> > > Jann pointed out that there are more syzbot reports related
> > > to a UAF in linkwatch:
> > >
> > > https://lore.kernel.org/netdev/?q=__linkwatch_run_queue+syzbot
> > >
> > > Third, I think an API which schedules work, invisibly to the driver,
> > > is dangerous and misguided. If it is illegal to call
> > > netif_carrier_on/off() for an unregistered but not yet freed netdev,
> > > catch that in core networking code and don't expect drivers to respect
> > > a rule which isn't even documented.
> >
> > Doesn't mean we should make it legal. We can add a warning to catch
> > abuses.
>
> That would be inconsequent, considering that netif_carrier_on/off()
> do not warn for a reg_state of NETREG_UNINITIALIZED.
>
Yes, only 1500 calls to audit ;)
I guess we could start adding WARN_ON_ONCE(), then wait for a few
syzbot/users reports to fix offenders...
commit b47300168e770b60ab96c8924854c3b0eb4260eb
Author: David S. Miller <davem@davemloft.net>
Date: Wed Nov 19 15:33:54 2008 -0800
net: Do not fire linkwatch events until the device is registered.
Several device drivers try to do things like netif_carrier_off()
before register_netdev() is invoked. This is bogus, but too many
drivers do this to fix them all up in one go.
Reported-by: Folkert van Heusden <folkert@vanheusden.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
^ permalink raw reply
* linux-next: Signed-off-by missing for commit in the net tree
From: Stephen Rothwell @ 2022-04-25 21:53 UTC (permalink / raw)
To: David Miller, Networking
Cc: Rongguang Wei, Pablo Neira Ayuso, Linux Kernel Mailing List,
Linux Next Mailing List
[-- Attachment #1: Type: text/plain, Size: 166 bytes --]
Hi all,
Commit
b9b1e0da5800 ("netfilter: flowtable: Remove the empty file")
is missing a Signed-off-by from its author.
--
Cheers,
Stephen Rothwell
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH 0/4] tools/bpf: allow building with musl
From: patchwork-bot+netdevbpf @ 2022-04-25 21:30 UTC (permalink / raw)
To: Dominique Martinet
Cc: bpf, netdev, linux-kernel, kpsingh, john.fastabend, yhs,
songliubraving, kafai, andrii, daniel, ast
In-Reply-To: <20220424051022.2619648-1-asmadeus@codewreck.org>
Hello:
This series was applied to bpf/bpf-next.git (master)
by Daniel Borkmann <daniel@iogearbox.net>:
On Sun, 24 Apr 2022 14:10:18 +0900 you wrote:
> Hi,
>
> I'd like to build bpftool on alpine linux, which is musl based.
>
> There are a few incompatibilities with it, I've commented on each patch
> when I could think of alternative solutions.
>
> [...]
Here is the summary with links:
- [1/4] tools/bpf/runqslower: musl compat: explicitly link with libargp if found
(no matching commit)
- [2/4] tools/bpf: musl compat: do not use DEFFILEMODE
(no matching commit)
- [3/4] tools/bpf: musl compat: replace nftw with FTW_ACTIONRETVAL
https://git.kernel.org/bpf/bpf-next/c/93bc2e9e943d
- [4/4] tools/bpf: replace sys/fcntl.h by fcntl.h
https://git.kernel.org/bpf/bpf-next/c/246bdfa52f33
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* [PATCH 2/2] net: remove comments that mention obsolete __SLOW_DOWN_IO
From: Bjorn Helgaas @ 2022-04-25 21:26 UTC (permalink / raw)
To: Jakub Kicinski, David S . Miller, Paolo Abeni, Chas Williams
Cc: linux-atm-general, netdev, linux-kernel, Bjorn Helgaas
In-Reply-To: <20220425212644.1659070-1-helgaas@kernel.org>
From: Bjorn Helgaas <bhelgaas@google.com>
The only remaining definitions of __SLOW_DOWN_IO (for alpha and ia64) do
nothing, and the only mentions in networking are in comments. Remove these
mentions.
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
drivers/atm/nicstarmac.c | 5 -----
drivers/net/ethernet/dec/tulip/winbond-840.c | 2 --
drivers/net/ethernet/natsemi/natsemi.c | 2 --
3 files changed, 9 deletions(-)
diff --git a/drivers/atm/nicstarmac.c b/drivers/atm/nicstarmac.c
index e0dda9062e6b..791f69a07ddf 100644
--- a/drivers/atm/nicstarmac.c
+++ b/drivers/atm/nicstarmac.c
@@ -14,11 +14,6 @@ typedef void __iomem *virt_addr_t;
#define CYCLE_DELAY 5
-/*
- This was the original definition
-#define osp_MicroDelay(microsec) \
- do { int _i = 4*microsec; while (--_i > 0) { __SLOW_DOWN_IO; }} while (0)
-*/
#define osp_MicroDelay(microsec) {unsigned long useconds = (microsec); \
udelay((useconds));}
/*
diff --git a/drivers/net/ethernet/dec/tulip/winbond-840.c b/drivers/net/ethernet/dec/tulip/winbond-840.c
index 86b1d23eba83..1db19463fd46 100644
--- a/drivers/net/ethernet/dec/tulip/winbond-840.c
+++ b/drivers/net/ethernet/dec/tulip/winbond-840.c
@@ -474,8 +474,6 @@ static int w840_probe1(struct pci_dev *pdev, const struct pci_device_id *ent)
No extra delay is needed with 33Mhz PCI, but future 66Mhz access may need
a delay. Note that pre-2.0.34 kernels had a cache-alignment bug that
made udelay() unreliable.
- The old method of using an ISA access as a delay, __SLOW_DOWN_IO__, is
- deprecated.
*/
#define eeprom_delay(ee_addr) ioread32(ee_addr)
diff --git a/drivers/net/ethernet/natsemi/natsemi.c b/drivers/net/ethernet/natsemi/natsemi.c
index 82a22711ce45..50bca486a244 100644
--- a/drivers/net/ethernet/natsemi/natsemi.c
+++ b/drivers/net/ethernet/natsemi/natsemi.c
@@ -989,8 +989,6 @@ static int natsemi_probe1(struct pci_dev *pdev, const struct pci_device_id *ent)
No extra delay is needed with 33Mhz PCI, but future 66Mhz access may need
a delay. Note that pre-2.0.34 kernels had a cache-alignment bug that
made udelay() unreliable.
- The old method of using an ISA access as a delay, __SLOW_DOWN_IO__, is
- deprecated.
*/
#define eeprom_delay(ee_addr) readl(ee_addr)
--
2.25.1
^ permalink raw reply related
* Re: [PATCH net-next RESEND] net: phy: marvell-88x2222: set proper phydev->port
From: Russell King (Oracle) @ 2022-04-25 21:33 UTC (permalink / raw)
To: Ivan Bornyakov
Cc: Andrew Lunn, Heiner Kallweit, David S. Miller, Jakub Kicinski,
Paolo Abeni, netdev, linux-kernel, system
In-Reply-To: <20220425041637.5946-1-i.bornyakov@metrotek.ru>
On Mon, Apr 25, 2022 at 07:16:37AM +0300, Ivan Bornyakov wrote:
> phydev->port was not set and always reported as PORT_TP.
> Set phydev->port according to inserted SFP module.
>
> Signed-off-by: Ivan Bornyakov <i.bornyakov@metrotek.ru>
> ---
> drivers/net/phy/marvell-88x2222.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/net/phy/marvell-88x2222.c b/drivers/net/phy/marvell-88x2222.c
> index ec4f1407a78c..9f971b37ec35 100644
> --- a/drivers/net/phy/marvell-88x2222.c
> +++ b/drivers/net/phy/marvell-88x2222.c
> @@ -603,6 +603,7 @@ static int mv2222_sfp_insert(void *upstream, const struct sfp_eeprom_id *id)
> dev = &phydev->mdio.dev;
>
> sfp_parse_support(phydev->sfp_bus, id, sfp_supported);
> + phydev->port = sfp_parse_port(phydev->sfp_bus, id, sfp_supported);
> sfp_interface = sfp_select_interface(phydev->sfp_bus, sfp_supported);
>
> dev_info(dev, "%s SFP module inserted\n", phy_modes(sfp_interface));
> @@ -639,6 +640,7 @@ static void mv2222_sfp_remove(void *upstream)
>
> priv->line_interface = PHY_INTERFACE_MODE_NA;
> linkmode_zero(priv->supported);
> + phydev->port = PORT_OTHER;
Can this PHY be used in dual-media mode, auto-switching between copper
and fibre? If so, is PORT_OTHER actually appropriate here, or should
the old value be saved when the module is inserted and restored when
it's removed?
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply
* Re: [PATCH 1/4] tools/bpf/runqslower: musl compat: explicitly link with libargp if found
From: Daniel Borkmann @ 2022-04-25 21:35 UTC (permalink / raw)
To: Dominique Martinet, bpf
Cc: netdev, linux-kernel, KP Singh, John Fastabend, Yonghong Song,
Song Liu, Martin KaFai Lau, Andrii Nakryiko, Alexei Starovoitov
In-Reply-To: <YmT1GxK1HimY2Os9@codewreck.org>
On 4/24/22 8:58 AM, Dominique Martinet wrote:
> Dominique Martinet wrote on Sun, Apr 24, 2022 at 02:10:19PM +0900:
>> After having done this work I noticed runqslower is not actually
>> installed, so ideally instead of all of this it'd make more sense to
>> just not build it: would it make sense to take it out of the defaults
>> build targets?
>> I could just directly build the appropriate targets from tools/bpf
>> directory with 'make bpftool bpf_dbg bpf_asm bpf_jit_disasm', but
>> ideally I'd like to keep alpine's build script way of calling make from
>> the tools parent directory, and 'make bpf' there is all or nothing.
>
> Well, it turns out runqslower doesn't build if the current kernel or
> vmlinux in tree don't have BTF enabled, so the current alpine builder
> can't build it.
>
> I've dropped this patch from my alpine MR[1] and built things directly
> with make bpftool etc as suggested above, so my suggestion to make it
> more easily buildable that way is probably the way to go?
> [1] https://gitlab.alpinelinux.org/alpine/aports/-/merge_requests/33554
Thanks for looking into this, Dominique! I slightly massaged patch 3 & 4
and applied it to bpf-next tree.
I don't really mind about patch 1 & 2, though out of tools/bpf/ the only
one you /really/ might want to package is bpftool. The other tools are on
the legacy side of things and JIT disasm you can also get via bpftool anyway.
Given this is not covered by BPF CI, are you planning to regularly check
for musl compatibility before a new kernel is cut?
Thanks,
Daniel
^ permalink raw reply
* Re: [PATCH] net: linkwatch: ignore events for unregistered netdevs
From: Lukas Wunner @ 2022-04-25 21:18 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Paolo Abeni, Oliver Neukum, David S. Miller, Jann Horn,
Oleksij Rempel, Eric Dumazet, netdev, linux-usb, Andrew Lunn,
Jacky Chou, Willy Tarreau, Lino Sanfilippo, Philipp Rosenberger,
Heiner Kallweit, Greg Kroah-Hartman
In-Reply-To: <20220425074146.1fa27d5f@kernel.org>
On Mon, Apr 25, 2022 at 07:41:46AM -0700, Jakub Kicinski wrote:
> On Sat, 23 Apr 2022 18:07:23 +0200 Lukas Wunner wrote:
> > > Looking at the original report it looks like the issue could be
> > > resolved with a more usb-specific change: e.g. it looks like
> > > usbnet_defer_kevent() is not acquiring a dev reference as it should.
> > >
> > > Have you considered that path?
> >
> > First of all, the diffstat of the patch shows this is an opportunity
> > to reduce LoC as well as simplify and speed up device teardown.
> >
> > Second, the approach you're proposing won't work if a driver calls
> > netif_carrier_on/off() after unregister_netdev().
> >
> > It seems prudent to prevent such a misbehavior in *any* driver,
> > not just usbnet. usbnet may not be the only one doing it wrong.
> > Jann pointed out that there are more syzbot reports related
> > to a UAF in linkwatch:
> >
> > https://lore.kernel.org/netdev/?q=__linkwatch_run_queue+syzbot
> >
> > Third, I think an API which schedules work, invisibly to the driver,
> > is dangerous and misguided. If it is illegal to call
> > netif_carrier_on/off() for an unregistered but not yet freed netdev,
> > catch that in core networking code and don't expect drivers to respect
> > a rule which isn't even documented.
>
> Doesn't mean we should make it legal. We can add a warning to catch
> abuses.
That would be inconsequent, considering that netif_carrier_on/off()
do not warn for a reg_state of NETREG_UNINITIALIZED.
Thanks,
Lukas
^ permalink raw reply
* Re: [PATCH 0/7] Remove unused SLOW_DOWN_IO
From: Bjorn Helgaas @ 2022-04-25 21:16 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Rich Felker, linux-ia64, Yoshinori Sato, linux-sh, linux-alpha,
linux-kernel, linux-atm-general, netdev, Ivan Kokshaysky,
Chas Williams, Bjorn Helgaas, Paul Mackerras, Matt Turner,
Paolo Abeni, linuxppc-dev, David S . Miller, Richard Henderson
In-Reply-To: <20220422104828.75c726d0@kernel.org>
On Fri, Apr 22, 2022 at 10:48:28AM -0700, Jakub Kicinski wrote:
> On Fri, 15 Apr 2022 14:08:10 -0500 Bjorn Helgaas wrote:
> > From: Bjorn Helgaas <bhelgaas@google.com>
> >
> > Only alpha, ia64, powerpc, and sh define SLOW_DOWN_IO, and there are no
> > actual uses of it. The few references to it are in situations that are
> > themselves unused. Remove them all.
> >
> > It should be safe to apply these independently and in any order. The only
> > place SLOW_DOWN_IO is used at all is the lmc_var.h definition of DELAY,
> > which is itself never used.
>
> Hi Bojrn! Would you mind reposting just patches 1 and 3 for networking?
> LMC got removed in net-next (commit a5b116a0fa90 ("net: wan: remove the
> lanmedia (lmc) driver")) so the entire series fails to apply and therefore
> defeats all of our patch handling scripts :S
Sure, coming up, with reduced cc: list.
^ permalink raw reply
* Re: [net-next v4 0/3] use standard sysctl macro
From: Luis Chamberlain @ 2022-04-25 20:53 UTC (permalink / raw)
To: Jakub Kicinski
Cc: xiangxia.m.yue, netdev, linux-fsdevel, Kees Cook, Iurii Zaikin,
David S. Miller, Paolo Abeni, Hideaki YOSHIFUJI, David Ahern,
Simon Horman, Julian Anastasov, Pablo Neira Ayuso,
Jozsef Kadlecsik, Florian Westphal, Shuah Khan, Andrew Morton,
Alexei Starovoitov, Eric Dumazet, Lorenz Bauer, Akhmat Karakotov
In-Reply-To: <20220425125644.52e3aad4@kernel.org>
On Mon, Apr 25, 2022 at 12:56:44PM -0700, Jakub Kicinski wrote:
> On Mon, 25 Apr 2022 12:47:06 -0700 Luis Chamberlain wrote:
> > I have a better option. I checked to see the diff stat between
> > the proposed patch to see what the chances of a conflict are
> > and so far I don't see any conflict so I think this patchset
> > should just go through your tree.
> >
> > So feel free to take it in! Let me know if that's OK!
>
> Ok, assuming the netfilter and bpf patches I saw were the only other
> conversions we can resolve the conflicts before code reaches Linus...
Sure thing.
Luis
^ permalink raw reply
* Re: [PATCH bpf-next] bpf: use bpf_prog_run_array_cg_flags everywhere
From: Martin KaFai Lau @ 2022-04-25 20:37 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: Andrii Nakryiko, Networking, bpf, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko
In-Reply-To: <CAKH8qBsTiQA5knxoBSqxCYav89QdSN0j6t1EWX1MEVbAqLj6kg@mail.gmail.com>
On Wed, Apr 20, 2022 at 03:30:43PM -0700, Stanislav Fomichev wrote:
> On Wed, Apr 20, 2022 at 3:04 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Tue, Apr 19, 2022 at 3:23 PM Stanislav Fomichev <sdf@google.com> wrote:
> > >
> > > Rename bpf_prog_run_array_cg_flags to bpf_prog_run_array_cg and
> > > use it everywhere. check_return_code already enforces sane
> > > return ranges for all cgroup types. (only egress and bind hooks have
> > > uncanonical return ranges, the rest is using [0, 1])
> > >
> > > No functional changes.
> > >
> > > Suggested-by: Alexei Starovoitov <ast@kernel.org>
> > > Signed-off-by: Stanislav Fomichev <sdf@google.com>
> > > ---
> > > include/linux/bpf-cgroup.h | 8 ++---
> > > kernel/bpf/cgroup.c | 70 ++++++++++++--------------------------
> > > 2 files changed, 24 insertions(+), 54 deletions(-)
> > >
> > > diff --git a/include/linux/bpf-cgroup.h b/include/linux/bpf-cgroup.h
> > > index 88a51b242adc..669d96d074ad 100644
> > > --- a/include/linux/bpf-cgroup.h
> > > +++ b/include/linux/bpf-cgroup.h
> > > @@ -225,24 +225,20 @@ static inline bool cgroup_bpf_sock_enabled(struct sock *sk,
> > >
> > > #define BPF_CGROUP_RUN_SA_PROG(sk, uaddr, atype) \
> > > ({ \
> > > - u32 __unused_flags; \
> > > int __ret = 0; \
> > > if (cgroup_bpf_enabled(atype)) \
> > > __ret = __cgroup_bpf_run_filter_sock_addr(sk, uaddr, atype, \
> > > - NULL, \
> > > - &__unused_flags); \
> > > + NULL, NULL); \
> > > __ret; \
> > > })
> > >
> > > #define BPF_CGROUP_RUN_SA_PROG_LOCK(sk, uaddr, atype, t_ctx) \
> > > ({ \
> > > - u32 __unused_flags; \
> > > int __ret = 0; \
> > > if (cgroup_bpf_enabled(atype)) { \
> > > lock_sock(sk); \
> > > __ret = __cgroup_bpf_run_filter_sock_addr(sk, uaddr, atype, \
> > > - t_ctx, \
> > > - &__unused_flags); \
> > > + t_ctx, NULL); \
> > > release_sock(sk); \
> > > } \
> > > __ret; \
> > > diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
> > > index 0cb6211fcb58..f61eca32c747 100644
> > > --- a/kernel/bpf/cgroup.c
> > > +++ b/kernel/bpf/cgroup.c
> > > @@ -25,50 +25,18 @@ EXPORT_SYMBOL(cgroup_bpf_enabled_key);
> > > /* __always_inline is necessary to prevent indirect call through run_prog
> > > * function pointer.
> > > */
> > > -static __always_inline int
> > > -bpf_prog_run_array_cg_flags(const struct cgroup_bpf *cgrp,
> > > - enum cgroup_bpf_attach_type atype,
> > > - const void *ctx, bpf_prog_run_fn run_prog,
> > > - int retval, u32 *ret_flags)
> > > -{
> > > - const struct bpf_prog_array_item *item;
> > > - const struct bpf_prog *prog;
> > > - const struct bpf_prog_array *array;
> > > - struct bpf_run_ctx *old_run_ctx;
> > > - struct bpf_cg_run_ctx run_ctx;
> > > - u32 func_ret;
> > > -
> > > - run_ctx.retval = retval;
> > > - migrate_disable();
> > > - rcu_read_lock();
> > > - array = rcu_dereference(cgrp->effective[atype]);
> > > - item = &array->items[0];
> > > - old_run_ctx = bpf_set_run_ctx(&run_ctx.run_ctx);
> > > - while ((prog = READ_ONCE(item->prog))) {
> > > - run_ctx.prog_item = item;
> > > - func_ret = run_prog(prog, ctx);
> > > - if (!(func_ret & 1) && !IS_ERR_VALUE((long)run_ctx.retval))
> > > - run_ctx.retval = -EPERM;
> > > - *(ret_flags) |= (func_ret >> 1);
> > > - item++;
> > > - }
> > > - bpf_reset_run_ctx(old_run_ctx);
> > > - rcu_read_unlock();
> > > - migrate_enable();
> > > - return run_ctx.retval;
> > > -}
> > > -
> > > static __always_inline int
> > > bpf_prog_run_array_cg(const struct cgroup_bpf *cgrp,
> > > enum cgroup_bpf_attach_type atype,
> > > const void *ctx, bpf_prog_run_fn run_prog,
> > > - int retval)
> > > + int retval, u32 *ret_flags)
> > > {
> > > const struct bpf_prog_array_item *item;
> > > const struct bpf_prog *prog;
> > > const struct bpf_prog_array *array;
> > > struct bpf_run_ctx *old_run_ctx;
> > > struct bpf_cg_run_ctx run_ctx;
> > > + u32 func_ret;
> > >
> > > run_ctx.retval = retval;
> > > migrate_disable();
> > > @@ -78,8 +46,11 @@ bpf_prog_run_array_cg(const struct cgroup_bpf *cgrp,
> > > old_run_ctx = bpf_set_run_ctx(&run_ctx.run_ctx);
> > > while ((prog = READ_ONCE(item->prog))) {
> > > run_ctx.prog_item = item;
> > > - if (!run_prog(prog, ctx) && !IS_ERR_VALUE((long)run_ctx.retval))
> > > + func_ret = run_prog(prog, ctx);
> > > + if (!(func_ret & 1) && !IS_ERR_VALUE((long)run_ctx.retval))
> >
> > to be completely true to previous behavior, shouldn't there be
> >
> > if (ret_flags)
> > func_ret &= 1;
> > if (!func_ret && !IS_ERR_VALUE(...))
> >
> > here?
> >
> > This might have been discussed previously and I missed it. If that's
> > so, please ignore.
>
> We are converting the cases where run_prog(prog, ctx) returns 0 or 1,
> so it seems like we don't have to reproduce the existing behavior
> 1-to-1?
> So I'm not sure it matters, or am I missing something?
A nit, how about testing 'if (ret_flags)' first such that
it is obvious which case will use higher bits in the return value.
The compiler may be able to optimize the ret_flags == NULL case also ?
Something like:
func_ret = run_prog(prog, ctx);
/* The cg bpf prog uses the higher bits of the return value */
if (ret_flags) {
*(ret_flags) |= (func_ret >> 1);
func_ret &= 1;
}
if (!func_ret && !IS_ERR_VALUE((long)run_ctx.retval))
run_ctx.retval = -EPERM;
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox