* Re: [PATCH wpan-next v2 05/11] net: mac802154: Bring the hability to hold the transmit queue
From: Alexander Aring @ 2022-05-17 13:19 UTC (permalink / raw)
To: Miquel Raynal
Cc: Alexander Aring, Stefan Schmidt, linux-wpan - ML, David S. Miller,
Jakub Kicinski, Paolo Abeni, open list:NETWORKING [GENERAL],
David Girault, Romuald Despres, Frederic Blain, Nicolas Schodet,
Thomas Petazzoni
In-Reply-To: <20220517112726.4b89e907@xps-13>
Hi,
On Tue, May 17, 2022 at 5:28 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> Hi Alex,
>
> > > @@ -84,7 +118,7 @@ void ieee802154_xmit_complete(struct ieee802154_hw *hw, struct sk_buff *skb,
> > > hw->phy->sifs_period * NSEC_PER_USEC,
> > > HRTIMER_MODE_REL);
> > > } else {
> > > - ieee802154_wake_queue(hw);
> > > + ieee802154_release_queue(local);
> > > }
> > >
> > > dev_consume_skb_any(skb);
> > > @@ -98,7 +132,7 @@ void ieee802154_xmit_error(struct ieee802154_hw *hw, struct sk_buff *skb,
> > > struct ieee802154_local *local = hw_to_local(hw);
> > >
> > > local->tx_result = reason;
> > > - ieee802154_wake_queue(hw);
> > > + ieee802154_release_queue(local);
> > > dev_kfree_skb_any(skb);
> > > atomic_dec(&hw->phy->ongoing_txs);
> >
> > I am pretty sure that will end in a scheduling while atomic warning
> > with hwsim. If you don't hit it you have the wrong config, you need to
> > enable such warnings and have the right preemption model setting.
>
> I was using the "desktop" kernel preemption model (voluntary), I've
> switched to CONFIG_PREEMPT ("Preemptible kernel (Low-latency)"),
> and enabled CONFIG_DEBUG_ATOMIC_SLEEP. You are right that we should use
> a spinlock instead of a mutex here. However I don't think disabling
> IRQs is necessary, so I'll switch to spin_(un)lock() calls.
>
In my opinion it's necessary for the ifs hrtimer. Normal
spin_lock/unlock is not the right fit here.
- Alex
^ permalink raw reply
* Re: [PATCH 14/30] panic: Properly identify the panic event to the notifiers' callbacks
From: Petr Mladek @ 2022-05-17 13:11 UTC (permalink / raw)
To: Guilherme G. Piccoli
Cc: akpm, bhe, kexec, linux-kernel, bcm-kernel-feedback-list,
linuxppc-dev, linux-alpha, linux-edac, linux-hyperv, linux-leds,
linux-mips, linux-parisc, linux-pm, linux-remoteproc, linux-s390,
linux-tegra, linux-um, linux-xtensa, netdev, openipmi-developer,
rcu, sparclinux, xen-devel, x86, kernel-dev, kernel, halves,
fabiomirmar, alejandro.j.jimenez, andriy.shevchenko, arnd, bp,
corbet, d.hatayama, dave.hansen, dyoung, feng.tang, gregkh,
mikelley, hidehiro.kawai.ez, jgross, john.ogness, keescook, luto,
mhiramat, mingo, paulmck, peterz, rostedt, senozhatsky, stern,
tglx, vgoyal, vkuznets, will
In-Reply-To: <244a412c-4589-28d1-bb77-d3648d4f0b12@igalia.com>
On Tue 2022-05-10 13:16:54, Guilherme G. Piccoli wrote:
> On 10/05/2022 12:16, Petr Mladek wrote:
> > [...]
> > Hmm, this looks like a hack. PANIC_UNUSED will never be used.
> > All notifiers will be always called with PANIC_NOTIFIER.
> >
> > The @val parameter is normally used when the same notifier_list
> > is used in different situations.
> >
> > But you are going to use it when the same notifier is used
> > in more lists. This is normally distinguished by the @nh
> > (atomic_notifier_head) parameter.
> >
> > IMHO, it is a bad idea. First, it would confuse people because
> > it does not follow the original design of the parameters.
> > Second, the related code must be touched anyway when
> > the notifier is moved into another list so it does not
> > help much.
> >
> > Or do I miss anything, please?
> >
> > Best Regards,
> > Petr
>
> Hi Petr, thanks for the review.
>
> I'm not strong attached to this patch, so we could drop it and refactor
> the code of next patches to use the @nh as identification - but
> personally, I feel this parameter could be used to identify the list
> that called such function, in other words, what is the event that
> triggered the callback. Some notifiers are even declared with this
> parameter called "ev", like the event that triggers the notifier.
>
>
> You mentioned 2 cases:
>
> (a) Same notifier_list used in different situations;
>
> (b) Same *notifier callback* used in different lists;
>
> Mine is case (b), right? Can you show me an example of case (a)?
There are many examples of case (a):
+ module_notify_list:
MODULE_STATE_LIVE, /* Normal state. */
MODULE_STATE_COMING, /* Full formed, running module_init. */
MODULE_STATE_GOING, /* Going away. */
MODULE_STATE_UNFORMED, /* Still setting it up. */
+ netdev_chain:
NETDEV_UP = 1, /* For now you can't veto a device up/down */
NETDEV_DOWN,
NETDEV_REBOOT, /* Tell a protocol stack a network interface
detected a hardware crash and restarted
- we can use this eg to kick tcp sessions
once done */
NETDEV_CHANGE, /* Notify device state change */
NETDEV_REGISTER,
NETDEV_UNREGISTER,
NETDEV_CHANGEMTU, /* notify after mtu change happened */
NETDEV_CHANGEADDR, /* notify after the address change */
NETDEV_PRE_CHANGEADDR, /* notify before the address change */
NETDEV_GOING_DOWN,
...
+ vt_notifier_list:
#define VT_ALLOCATE 0x0001 /* Console got allocated */
#define VT_DEALLOCATE 0x0002 /* Console will be deallocated */
#define VT_WRITE 0x0003 /* A char got output */
#define VT_UPDATE 0x0004 /* A bigger update occurred */
#define VT_PREWRITE 0x0005 /* A char is about to be written to the console */
+ die_chain:
DIE_OOPS = 1,
DIE_INT3,
DIE_DEBUG,
DIE_PANIC,
DIE_NMI,
DIE_DIE,
DIE_KERNELDEBUG,
...
These all call the same list/chain in different situations.
The situation is distinguished by @val.
> You can see in the following patches (or grep the kernel) that people are using
> this identification parameter to determine which kind of OOPS trigger
> the callback to condition the execution of the function to specific
> cases.
Could you please show me some existing code for case (b)?
I am not able to find any except in your patches.
Anyway, the solution in 16th patch is bad, definitely.
hv_die_panic_notify_crash() uses "val" to disinguish
both:
+ "panic_notifier_list" vs "die_chain"
+ die_val when callen via "die_chain"
The API around "die_chain" API is not aware of enum panic_notifier_val
and the API using "panic_notifier_list" is not aware of enum die_val.
As I said, it is mixing apples and oranges and it is error prone.
Best Regards,
Petr
^ permalink raw reply
* Re: [PATCH 05/30] misc/pvpanic: Convert regular spinlock into trylock on panic path
From: Guilherme G. Piccoli @ 2022-05-17 13:03 UTC (permalink / raw)
To: Petr Mladek
Cc: akpm, bhe, kexec, linux-kernel, bcm-kernel-feedback-list,
linuxppc-dev, linux-alpha, linux-arm-kernel, linux-edac,
linux-hyperv, linux-leds, linux-mips, linux-parisc, linux-pm,
linux-remoteproc, linux-s390, linux-tegra, linux-um, linux-xtensa,
netdev, openipmi-developer, rcu, sparclinux, xen-devel, x86,
kernel-dev, kernel, halves, fabiomirmar, alejandro.j.jimenez,
andriy.shevchenko, arnd, bp, corbet, d.hatayama, dave.hansen,
dyoung, feng.tang, gregkh, mikelley, hidehiro.kawai.ez, jgross,
john.ogness, keescook, luto, mhiramat, mingo, paulmck, peterz,
rostedt, senozhatsky, stern, tglx, vgoyal, vkuznets, will,
Christophe JAILLET, Mihai Carabas, Shile Zhang, Wang ShaoBo,
zhenwei pi
In-Reply-To: <YoN/x2fpdDU4+nSB@alley>
On 17/05/2022 07:58, Petr Mladek wrote:
> [...]
>> Thanks for the review Petr. Patch was already merged - my goal was to be
>> concise, i.e., a patch per driver / module, so the patch kinda fixes
>> whatever I think is wrong with the driver with regards panic handling.
>>
>> Do you think it worth to remove this patch from Greg's branch just to
>> split it in 2? Personally I think it's not worth, but opinions are welcome.
>
> No problem. It is not worth the effort.
>
OK, perfect!
Cheers,
Guilherme
^ permalink raw reply
* Re: [PATCH v2,bpf-next] samples/bpf: check detach prog exist or not in xdp_fwd
From: Toke Høiland-Jørgensen @ 2022-05-17 12:57 UTC (permalink / raw)
To: Zhengchao Shao, bpf, netdev, linux-kernel, ast, daniel, davem,
kuba, hawk, john.fastabend, andrii, kafai, songliubraving, yhs,
kpsingh
Cc: weiyongjun1, shaozhengchao, yuehaibing
In-Reply-To: <20220517112748.358295-1-shaozhengchao@huawei.com>
Zhengchao Shao <shaozhengchao@huawei.com> writes:
> Before detach the prog, we should check detach prog exist or not.
>
> Signed-off-by: Zhengchao Shao <shaozhengchao@huawei.com>
> ---
> samples/bpf/xdp_fwd_user.c | 52 +++++++++++++++++++++++++++++++-------
> 1 file changed, 43 insertions(+), 9 deletions(-)
>
> diff --git a/samples/bpf/xdp_fwd_user.c b/samples/bpf/xdp_fwd_user.c
> index 1828487bae9a..2294486ef10a 100644
> --- a/samples/bpf/xdp_fwd_user.c
> +++ b/samples/bpf/xdp_fwd_user.c
> @@ -47,17 +47,51 @@ static int do_attach(int idx, int prog_fd, int map_fd, const char *name)
> return err;
> }
>
> -static int do_detach(int idx, const char *name)
> +static int do_detach(int idx, const char *name, const char *prog_name)
two 'name' arguments is a bit confusing; could we rename the parameters
to 'ifindex', 'ifname' and 'app_name', then use 'prog_name' for the
stack variable below instead of 'namepad'?
> {
> - int err;
> + int err = 1;
> + __u32 info_len, curr_prog_id;
> + struct bpf_prog_info prog_info = {};
> + int prog_fd;
> + char namepad[BPF_OBJ_NAME_LEN];
Reverse x-mas tree, please.
> +
> + if (bpf_xdp_query_id(idx, xdp_flags, &curr_prog_id)) {
> + printf("ERROR: bpf_xdp_query_id failed\n");
strerror(errno) might be nice to add to the error message, so users have
an inkling as to why the call is failing (same below).
> + return err;
> + }
> +
> + if (!curr_prog_id) {
> + printf("ERROR: flags(0x%x) xdp prog is not attached to %s\n",
> + xdp_flags, name);
> + return err;
> + }
>
> - err = bpf_xdp_detach(idx, xdp_flags, NULL);
> - if (err < 0)
> - printf("ERROR: failed to detach program from %s\n", name);
> + info_len = sizeof(prog_info);
> + prog_fd = bpf_prog_get_fd_by_id(curr_prog_id);
> + if (prog_fd < 0 && errno == ENOENT) {
Why the ENOENT check? This should error out regardless of the errno, no?
> + printf("ERROR: bpf_prog_get_fd_by_id failed\n");
strerror(errno)
> + return err;
> + }
> +
> + err = bpf_obj_get_info_by_fd(prog_fd, &prog_info, &info_len);
> + if (err) {
> + printf("ERROR: bpf_obj_get_info_by_fd failed\n");
strerror(errno)
> + return err;
> + }
> + snprintf(namepad, sizeof(namepad), "%s_prog", prog_name);
If the binary somehow gets renamed, this may overflow and you'll end up
without a NULL byte terminating the string. So either check the input
length first, or make sure to set the last byte to '\0' after this
call...
> +
> + if (strcmp(prog_info.name, namepad)) {
> + printf("ERROR: %s isn't attached to %s\n", prog_name, name);
> + } else {
> + err = bpf_xdp_detach(idx, xdp_flags, NULL);
This call should be including an opts struct with the fd obtained above
as the old_prog_fd (so make sure it wasn't swapped out since the check).
> + if (err < 0)
> + printf("ERROR: failed to detach program from %s\n",
> + name);
strerror(errno)
-Toke
^ permalink raw reply
* Re: [PATCH v3 3/4] can: skb:: move can_dropped_invalid_skb and can_skb_headroom_valid to skb.c
From: Max Staudt @ 2022-05-17 12:39 UTC (permalink / raw)
To: Marc Kleine-Budde
Cc: Oliver Hartkopp, Vincent MAILHOL, linux-can, linux-kernel, netdev
In-Reply-To: <20220517122153.4r6n6kkbdslsa2hv@pengutronix.de>
On Tue, 17 May 2022 14:21:53 +0200
Marc Kleine-Budde <mkl@pengutronix.de> wrote:
> On 17.05.2022 14:14:04, Max Staudt wrote:
> > > After looking through drivers/net/can/Kconfig I would probably
> > > phrase it like this:
> > >
> > > Select CAN devices (hw/sw) -> we compile a can_dev module. E.g.
> > > to handle the skb stuff for vcan's.
> > >
> > > Select hardware CAN devices -> we compile the netlink stuff into
> > > can_dev and offer CAN_CALC_BITTIMING and CAN_LEDS to be compiled
> > > into can_dev too.
> > >
> > > In the latter case: The selection of flexcan, ti_hecc and
> > > mcp251xfd automatically selects CAN_RX_OFFLOAD which is then also
> > > compiled into can_dev.
> > >
> > > Would that fit in terms of complexity?
> >
> > IMHO these should always be compiled into can-dev. Out of tree
> > drivers are fairly common here, and having to determine which kind
> > of can-dev (stripped or not) the user has on their system is a
> > nightmare waiting to happen.
>
> I personally don't care about out-of-tree drivers.
I know that this is the official stance in the kernel.
But out-of-tree drivers do happen on a regular basis, even when
developing with the aim of upstreaming. And if a developer builds a
minimal kernel to host a CAN driver, without building in-tree hardware
CAN drivers, then can-dev will be there but behave differently from
can-dev in a full distro. Leading to heisenbugs and wasting time. The
source of heisenbugs really are the suggested *hidden* Kconfigs.
On another note, is the module accounting overhead in the kernel for
two new modules with relatively little code in each, code that almost
always is loaded when CAN is used, really worth it?
Okay, I think I'm out of 2 cent pieces now :)
Max
^ permalink raw reply
* Re: [PATCH bpf-next v3 4/4] bpf_trace: pass array of u64 values in kprobe_multi.addrs
From: Eugene Syromiatnikov @ 2022-05-17 12:30 UTC (permalink / raw)
To: Jiri Olsa
Cc: Masami Hiramatsu, Steven Rostedt, Ingo Molnar, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Song Liu,
Yonghong Song, John Fastabend, KP Singh, netdev, bpf,
linux-kernel, Shuah Khan, linux-kselftest
In-Reply-To: <YoNnAgDsIWef82is@krava>
On Tue, May 17, 2022 at 11:12:34AM +0200, Jiri Olsa wrote:
> On Tue, May 17, 2022 at 09:36:47AM +0200, Eugene Syromiatnikov wrote:
> > With the interface as defined, it is impossible to pass 64-bit kernel
> > addresses from a 32-bit userspace process in BPF_LINK_TYPE_KPROBE_MULTI,
> > which severly limits the useability of the interface, change the ABI
> > to accept an array of u64 values instead of (kernel? user?) longs.
> > Interestingly, the rest of the libbpf infrastructure uses 64-bit values
> > for kallsyms addresses already, so this patch also eliminates
> > the sym_addr cast in tools/lib/bpf/libbpf.c:resolve_kprobe_multi_cb().
>
> so the problem is when we have 32bit user sace on 64bit kernel right?
>
> I think we should keep addrs as longs in uapi and have kernel to figure out
> if it needs to read u32 or u64, like you did for symbols in previous patch
No, it's not possible here, as addrs are kernel addrs and not user space
addrs, so user space has to explicitly pass 64-bit addresses on 64-bit
kernels (or have a notion whether it is running on a 64-bit
or 32-bit kernel, and form the passed array accordingly, which is against
the idea of compat layer that tries to abstract it out).
> we'll need to fix also bpf_kprobe_multi_cookie_swap because it assumes
> 64bit user space pointers
>
> would be gret if we could have selftest for this
>
> thanks,
> jirka
>
> >
> > Fixes: 0dcac272540613d4 ("bpf: Add multi kprobe link")
> > Fixes: 5117c26e877352bc ("libbpf: Add bpf_link_create support for multi kprobes")
> > Fixes: ddc6b04989eb0993 ("libbpf: Add bpf_program__attach_kprobe_multi_opts function")
> > Fixes: f7a11eeccb111854 ("selftests/bpf: Add kprobe_multi attach test")
> > Fixes: 9271a0c7ae7a9147 ("selftests/bpf: Add attach test for bpf_program__attach_kprobe_multi_opts")
> > Fixes: 2c6401c966ae1fbe ("selftests/bpf: Add kprobe_multi bpf_cookie test")
> > Signed-off-by: Eugene Syromiatnikov <esyr@redhat.com>
> > ---
> > kernel/trace/bpf_trace.c | 25 ++++++++++++++++++----
> > tools/lib/bpf/bpf.h | 2 +-
> > tools/lib/bpf/libbpf.c | 8 +++----
> > tools/lib/bpf/libbpf.h | 2 +-
> > .../testing/selftests/bpf/prog_tests/bpf_cookie.c | 2 +-
> > .../selftests/bpf/prog_tests/kprobe_multi_test.c | 8 +++----
> > 6 files changed, 32 insertions(+), 15 deletions(-)
> >
> > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> > index 9d3028a..30a15b3 100644
> > --- a/kernel/trace/bpf_trace.c
> > +++ b/kernel/trace/bpf_trace.c
> > @@ -2454,7 +2454,7 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
> > void __user *ucookies;
> > unsigned long *addrs;
> > u32 flags, cnt, size, cookies_size;
> > - void __user *uaddrs;
> > + u64 __user *uaddrs;
> > u64 *cookies = NULL;
> > void __user *usyms;
> > int err;
> > @@ -2486,9 +2486,26 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
> > return -ENOMEM;
> >
> > if (uaddrs) {
> > - if (copy_from_user(addrs, uaddrs, size)) {
> > - err = -EFAULT;
> > - goto error;
> > + if (sizeof(*addrs) == sizeof(*uaddrs)) {
> > + if (copy_from_user(addrs, uaddrs, size)) {
> > + err = -EFAULT;
> > + goto error;
> > + }
> > + } else {
> > + u32 i;
> > + u64 addr;
> > +
> > + for (i = 0; i < cnt; i++) {
> > + if (get_user(addr, uaddrs + i)) {
> > + err = -EFAULT;
> > + goto error;
> > + }
> > + if (addr > ULONG_MAX) {
> > + err = -EINVAL;
> > + goto error;
> > + }
> > + addrs[i] = addr;
> > + }
> > }
> > } else {
> > struct user_syms us;
> > diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
> > index 2e0d373..da9c6037 100644
> > --- a/tools/lib/bpf/bpf.h
> > +++ b/tools/lib/bpf/bpf.h
> > @@ -418,7 +418,7 @@ struct bpf_link_create_opts {
> > __u32 flags;
> > __u32 cnt;
> > const char **syms;
> > - const unsigned long *addrs;
> > + const __u64 *addrs;
> > const __u64 *cookies;
> > } kprobe_multi;
> > struct {
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index ef7f302..35fa9c5 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -10737,7 +10737,7 @@ static bool glob_match(const char *str, const char *pat)
> >
> > struct kprobe_multi_resolve {
> > const char *pattern;
> > - unsigned long *addrs;
> > + __u64 *addrs;
> > size_t cap;
> > size_t cnt;
> > };
> > @@ -10752,12 +10752,12 @@ resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
> > if (!glob_match(sym_name, res->pattern))
> > return 0;
> >
> > - err = libbpf_ensure_mem((void **) &res->addrs, &res->cap, sizeof(unsigned long),
> > + err = libbpf_ensure_mem((void **) &res->addrs, &res->cap, sizeof(__u64),
> > res->cnt + 1);
> > if (err)
> > return err;
> >
> > - res->addrs[res->cnt++] = (unsigned long) sym_addr;
> > + res->addrs[res->cnt++] = sym_addr;
> > return 0;
> > }
> >
> > @@ -10772,7 +10772,7 @@ bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
> > };
> > struct bpf_link *link = NULL;
> > char errmsg[STRERR_BUFSIZE];
> > - const unsigned long *addrs;
> > + const __u64 *addrs;
> > int err, link_fd, prog_fd;
> > const __u64 *cookies;
> > const char **syms;
> > diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> > index 9e9a3fd..76e171d 100644
> > --- a/tools/lib/bpf/libbpf.h
> > +++ b/tools/lib/bpf/libbpf.h
> > @@ -489,7 +489,7 @@ struct bpf_kprobe_multi_opts {
> > /* array of function symbols to attach */
> > const char **syms;
> > /* array of function addresses to attach */
> > - const unsigned long *addrs;
> > + const __u64 *addrs;
> > /* array of user-provided values fetchable through bpf_get_attach_cookie */
> > const __u64 *cookies;
> > /* number of elements in syms/addrs/cookies arrays */
> > diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c b/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c
> > index 83ef55e3..e843840 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c
> > @@ -140,7 +140,7 @@ static void kprobe_multi_link_api_subtest(void)
> > cookies[6] = 7;
> > cookies[7] = 8;
> >
> > - opts.kprobe_multi.addrs = (const unsigned long *) &addrs;
> > + opts.kprobe_multi.addrs = (const __u64 *) &addrs;
> > opts.kprobe_multi.cnt = ARRAY_SIZE(addrs);
> > opts.kprobe_multi.cookies = (const __u64 *) &cookies;
> > prog_fd = bpf_program__fd(skel->progs.test_kprobe);
> > diff --git a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
> > index 586dc52..7646112 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
> > @@ -108,7 +108,7 @@ static void test_link_api_addrs(void)
> > GET_ADDR("bpf_fentry_test7", addrs[6]);
> > GET_ADDR("bpf_fentry_test8", addrs[7]);
> >
> > - opts.kprobe_multi.addrs = (const unsigned long*) addrs;
> > + opts.kprobe_multi.addrs = (const __u64 *) addrs;
> > opts.kprobe_multi.cnt = ARRAY_SIZE(addrs);
> > test_link_api(&opts);
> > }
> > @@ -186,7 +186,7 @@ static void test_attach_api_addrs(void)
> > GET_ADDR("bpf_fentry_test7", addrs[6]);
> > GET_ADDR("bpf_fentry_test8", addrs[7]);
> >
> > - opts.addrs = (const unsigned long *) addrs;
> > + opts.addrs = (const __u64 *) addrs;
> > opts.cnt = ARRAY_SIZE(addrs);
> > test_attach_api(NULL, &opts);
> > }
> > @@ -244,7 +244,7 @@ static void test_attach_api_fails(void)
> > goto cleanup;
> >
> > /* fail_2 - both addrs and syms set */
> > - opts.addrs = (const unsigned long *) addrs;
> > + opts.addrs = (const __u64 *) addrs;
> > opts.syms = syms;
> > opts.cnt = ARRAY_SIZE(syms);
> > opts.cookies = NULL;
> > @@ -258,7 +258,7 @@ static void test_attach_api_fails(void)
> > goto cleanup;
> >
> > /* fail_3 - pattern and addrs set */
> > - opts.addrs = (const unsigned long *) addrs;
> > + opts.addrs = (const __u64 *) addrs;
> > opts.syms = NULL;
> > opts.cnt = ARRAY_SIZE(syms);
> > opts.cookies = NULL;
> > --
> > 2.1.4
> >
>
^ permalink raw reply
* Re: [PATCH net] NFC: hci: fix sleep in atomic context bugs in nfc_hci_hcp_message_tx
From: duoming @ 2022-05-17 12:24 UTC (permalink / raw)
To: Krzysztof Kozlowski; +Cc: linux-kernel, netdev
In-Reply-To: <171c13bb-9fc9-0807-e872-6859dfa2603d@linaro.org>
Hello,
On Tue, 17 May 2022 13:39:44 +0200 wrote:
> You sent v2 one minute before replying here... that's not how discussion
> work. Please do not sent next version before reaching some consensus.
I am sorry. Before reaching some consensus, I will not send next version in the future.
Thanks for your guidance.
Best regards,
Duoming Zhou
^ permalink raw reply
* Re: [PATCH v3 3/4] can: skb:: move can_dropped_invalid_skb and can_skb_headroom_valid to skb.c
From: Marc Kleine-Budde @ 2022-05-17 12:21 UTC (permalink / raw)
To: Max Staudt
Cc: Oliver Hartkopp, Vincent MAILHOL, linux-can, linux-kernel, netdev
In-Reply-To: <20220517141404.578d188a.max@enpas.org>
[-- Attachment #1: Type: text/plain, Size: 1214 bytes --]
On 17.05.2022 14:14:04, Max Staudt wrote:
> > After looking through drivers/net/can/Kconfig I would probably phrase
> > it like this:
> >
> > Select CAN devices (hw/sw) -> we compile a can_dev module. E.g. to
> > handle the skb stuff for vcan's.
> >
> > Select hardware CAN devices -> we compile the netlink stuff into
> > can_dev and offer CAN_CALC_BITTIMING and CAN_LEDS to be compiled into
> > can_dev too.
> >
> > In the latter case: The selection of flexcan, ti_hecc and mcp251xfd
> > automatically selects CAN_RX_OFFLOAD which is then also compiled into
> > can_dev.
> >
> > Would that fit in terms of complexity?
>
> IMHO these should always be compiled into can-dev. Out of tree drivers
> are fairly common here, and having to determine which kind of can-dev
> (stripped or not) the user has on their system is a nightmare waiting to
> happen.
I personally don't care about out-of-tree drivers.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung West/Dortmund | Phone: +49-231-2826-924 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v3 3/4] can: skb:: move can_dropped_invalid_skb and can_skb_headroom_valid to skb.c
From: Max Staudt @ 2022-05-17 12:14 UTC (permalink / raw)
To: Oliver Hartkopp
Cc: Marc Kleine-Budde, Vincent MAILHOL, linux-can, linux-kernel,
netdev
In-Reply-To: <e054f6d4-7ed1-98ac-8364-425f4ef0f760@hartkopp.net>
On Tue, 17 May 2022 13:51:57 +0200
Oliver Hartkopp <socketcan@hartkopp.net> wrote:
> After looking through drivers/net/can/Kconfig I would probably phrase
> it like this:
>
> Select CAN devices (hw/sw) -> we compile a can_dev module. E.g. to
> handle the skb stuff for vcan's.
>
> Select hardware CAN devices -> we compile the netlink stuff into
> can_dev and offer CAN_CALC_BITTIMING and CAN_LEDS to be compiled into
> can_dev too.
>
> In the latter case: The selection of flexcan, ti_hecc and mcp251xfd
> automatically selects CAN_RX_OFFLOAD which is then also compiled into
> can_dev.
>
> Would that fit in terms of complexity?
IMHO these should always be compiled into can-dev. Out of tree drivers
are fairly common here, and having to determine which kind of can-dev
(stripped or not) the user has on their system is a nightmare waiting to
happen.
Max
^ permalink raw reply
* Re: [PATCH v3 3/4] can: skb:: move can_dropped_invalid_skb and can_skb_headroom_valid to skb.c
From: Oliver Hartkopp @ 2022-05-17 11:51 UTC (permalink / raw)
To: Marc Kleine-Budde, Vincent MAILHOL
Cc: linux-can, linux-kernel, Max Staudt, netdev
In-Reply-To: <20220517104545.eslountqjppvcnz2@pengutronix.de>
On 17.05.22 12:45, Marc Kleine-Budde wrote:
> On 17.05.2022 16:04:53, Vincent MAILHOL wrote:
>> So slcan, v(x)can and can-dev will select can-skb, and some of the
>> hardware drivers (still have to figure out the list) will select
>> can-rx-offload (other dependencies will stay as it is today).
>
> For rx-offload that's flexcan, ti_hecc and mcp251xfd
>
>> I think that splitting the current can-dev into can-skb + can-dev +
>> can-rx-offload is enough. Please let me know if you see a need for
>> more.
After looking through drivers/net/can/Kconfig I would probably phrase it
like this:
Select CAN devices (hw/sw) -> we compile a can_dev module. E.g. to
handle the skb stuff for vcan's.
Select hardware CAN devices -> we compile the netlink stuff into can_dev
and offer CAN_CALC_BITTIMING and CAN_LEDS to be compiled into can_dev too.
In the latter case: The selection of flexcan, ti_hecc and mcp251xfd
automatically selects CAN_RX_OFFLOAD which is then also compiled into
can_dev.
Would that fit in terms of complexity?
Best,
Oliver
^ permalink raw reply
* Re: [PATCH net-next] net: ifdefy the wireless pointers in struct net_device
From: Sven Eckelmann @ 2022-05-17 11:49 UTC (permalink / raw)
To: davem, Jakub Kicinski
Cc: netdev, edumazet, pabeni, Jakub Kicinski, johannes, alex.aring,
stefan, mareklindner, sw, a, linux-wireless, linux-wpan
In-Reply-To: <20220516215638.1787257-1-kuba@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 2686 bytes --]
On Monday, 16 May 2022 23:56:38 CEST Jakub Kicinski wrote:
> diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c
> index 83fb51b6e299..15d2bb4cd301 100644
> --- a/net/batman-adv/hard-interface.c
> +++ b/net/batman-adv/hard-interface.c
> @@ -307,9 +307,11 @@ static bool batadv_is_cfg80211_netdev(struct net_device *net_device)
> if (!net_device)
> return false;
>
> +#if IS_ENABLED(CONFIG_WIRELESS)
> /* cfg80211 drivers have to set ieee80211_ptr */
> if (net_device->ieee80211_ptr)
> return true;
> +#endif
>
> return false;
> }
Acked-by: Sven Eckelmann <sven@narfation.org>
On Tuesday, 17 May 2022 09:48:24 CEST Johannes Berg wrote:
> Something like the patch below might do that, but I haven't carefully
> checked it yet, nor checked if there are any paths in mac80211/drivers
> that might be doing this check - and it looks from Jakub's patch that
> batman code would like to check this too.
Yes, if something like netdev_is_wireless would be available then we could
change it to:
diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c
index 35fadb924849..50a53e3364bf 100644
--- a/net/batman-adv/hard-interface.c
+++ b/net/batman-adv/hard-interface.c
@@ -294,26 +294,6 @@ static bool batadv_is_wext_netdev(struct net_device *net_device)
return false;
}
-/**
- * batadv_is_cfg80211_netdev() - check if the given net_device struct is a
- * cfg80211 wifi interface
- * @net_device: the device to check
- *
- * Return: true if the net device is a cfg80211 wireless device, false
- * otherwise.
- */
-static bool batadv_is_cfg80211_netdev(struct net_device *net_device)
-{
- if (!net_device)
- return false;
-
- /* cfg80211 drivers have to set ieee80211_ptr */
- if (net_device->ieee80211_ptr)
- return true;
-
- return false;
-}
-
/**
* batadv_wifi_flags_evaluate() - calculate wifi flags for net_device
* @net_device: the device to check
@@ -328,7 +308,7 @@ static u32 batadv_wifi_flags_evaluate(struct net_device *net_device)
if (batadv_is_wext_netdev(net_device))
wifi_flags |= BATADV_HARDIF_WIFI_WEXT_DIRECT;
- if (batadv_is_cfg80211_netdev(net_device))
+ if (netdev_is_wireless(net_device))
wifi_flags |= BATADV_HARDIF_WIFI_CFG80211_DIRECT;
real_netdev = batadv_get_real_netdevice(net_device);
@@ -341,7 +321,7 @@ static u32 batadv_wifi_flags_evaluate(struct net_device *net_device)
if (batadv_is_wext_netdev(real_netdev))
wifi_flags |= BATADV_HARDIF_WIFI_WEXT_INDIRECT;
- if (batadv_is_cfg80211_netdev(real_netdev))
+ if (netdev_is_wireless(real_netdev))
wifi_flags |= BATADV_HARDIF_WIFI_CFG80211_INDIRECT;
out:
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* Re: [PATCH v2] net: phy: marvell: Add errata section 5.1 for Alaska PHY
From: Paolo Abeni @ 2022-05-17 11:45 UTC (permalink / raw)
To: Stefan Roese, netdev
Cc: Leszek Polak, Marek Behún, Andrew Lunn, Heiner Kallweit,
Russell King, David S . Miller
In-Reply-To: <c8e782cb-49cc-e792-9573-8fd2e5515c50@denx.de>
On Tue, 2022-05-17 at 13:21 +0200, Stefan Roese wrote:
> On 17.05.22 13:01, Paolo Abeni wrote:
> > On Mon, 2022-05-16 at 09:08 +0200, Stefan Roese wrote:
> > > From: Leszek Polak <lpolak@arri.de>
> > >
> > > As per Errata Section 5.1, if EEE is intended to be used, some register
> > > writes must be done once after every hardware reset. This patch now adds
> > > the necessary register writes as listed in the Marvell errata.
> > >
> > > Without this fix we experience ethernet problems on some of our boards
> > > equipped with a new version of this ethernet PHY (different supplier).
> > >
> > > The fix applies to Marvell Alaska 88E1510/88E1518/88E1512/88E1514
> > > Rev. A0.
> > >
> > > Signed-off-by: Leszek Polak <lpolak@arri.de>
> > > Signed-off-by: Stefan Roese <sr@denx.de>
> > > Cc: Marek Behún <kabel@kernel.org>
> > > Cc: Andrew Lunn <andrew@lunn.ch>
> > > Cc: Heiner Kallweit <hkallweit1@gmail.com>
> > > Cc: Russell King <linux@armlinux.org.uk>
> > > Cc: David S. Miller <davem@davemloft.net>
> >
> > It's not clear to me if you are targeting -net or net-next, could you
> > please clarify? In case this is for -net, please add a suitable fixes
> > tag, thanks!
>
> Sorry for not being clear on this. net-next is good AFAICT.
>
> Should I re-submit to net-next?
Not needed, I'm applying it.
Thanks!
Paolo
^ permalink raw reply
* Re: [PATCH net v2] NFC: hci: fix sleep in atomic context bugs in nfc_hci_hcp_message_tx
From: Krzysztof Kozlowski @ 2022-05-17 11:42 UTC (permalink / raw)
To: Duoming Zhou, linux-kernel
Cc: kuba, davem, edumazet, pabeni, gregkh, alexander.deucher, broonie,
netdev
In-Reply-To: <20220517105526.114421-1-duoming@zju.edu.cn>
On 17/05/2022 12:55, Duoming Zhou wrote:
> There are sleep in atomic context bugs when the request to secure
> element of st21nfca is timeout. The root cause is that kzalloc and
> alloc_skb with GFP_KERNEL parameter and mutex_lock are called in
> st21nfca_se_wt_timeout which is a timer handler. The call tree shows
> the execution paths that could lead to bugs:
>
> (Interrupt context)
> st21nfca_se_wt_timeout
> nfc_hci_send_event
> nfc_hci_hcp_message_tx
> kzalloc(..., GFP_KERNEL) //may sleep
> alloc_skb(..., GFP_KERNEL) //may sleep
> mutex_lock() //may sleep
>
> This patch changes allocation mode of kzalloc and alloc_skb from
> GFP_KERNEL to GFP_ATOMIC and changes mutex_lock to spin_lock in
> order to prevent atomic context from sleeping.
>
> Fixes: 2130fb97fecf ("NFC: st21nfca: Adding support for secure element")
> Signed-off-by: Duoming Zhou <duoming@zju.edu.cn>
> ---
> Changes in v2:
> - Change mutex_lock to spin_lock.
>
> include/net/nfc/hci.h | 3 ++-
> net/nfc/hci/core.c | 18 +++++++++---------
> net/nfc/hci/hcp.c | 10 +++++-----
> 3 files changed, 16 insertions(+), 15 deletions(-)
>
> diff --git a/include/net/nfc/hci.h b/include/net/nfc/hci.h
> index 756c11084f6..8f66e6e6b91 100644
> --- a/include/net/nfc/hci.h
> +++ b/include/net/nfc/hci.h
> @@ -103,7 +103,8 @@ struct nfc_hci_dev {
>
> bool shutting_down;
>
> - struct mutex msg_tx_mutex;
> + /* The spinlock is used to protect resources related with hci message TX */
> + spinlock_t msg_tx_spin;
>
> struct list_head msg_tx_queue;
>
> diff --git a/net/nfc/hci/core.c b/net/nfc/hci/core.c
> index ceb87db57cd..fa22f9fe5fc 100644
> --- a/net/nfc/hci/core.c
> +++ b/net/nfc/hci/core.c
> @@ -68,7 +68,7 @@ static void nfc_hci_msg_tx_work(struct work_struct *work)
> struct sk_buff *skb;
> int r = 0;
>
> - mutex_lock(&hdev->msg_tx_mutex);
> + spin_lock(&hdev->msg_tx_spin);
> if (hdev->shutting_down)
> goto exit;
How did you test your patch?
Did you check, really check, that this can be an atomic (non-sleeping)
section?
I have doubts because I found at least one path leading to device_lock
(which is a mutex) called within your new code.
Before sending a new version, please wait for discussion to reach some
consensus. The quality of these fixes is really poor. :(
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH net] NFC: hci: fix sleep in atomic context bugs in nfc_hci_hcp_message_tx
From: Krzysztof Kozlowski @ 2022-05-17 11:39 UTC (permalink / raw)
To: duoming
Cc: linux-kernel, davem, edumazet, kuba, pabeni, gregkh,
alexander.deucher, broonie, netdev
In-Reply-To: <fc6a78c.196ab.180d1a98cc9.Coremail.duoming@zju.edu.cn>
On 17/05/2022 12:56, duoming@zju.edu.cn wrote:
> Hello,
>
> On Mon, 16 May 2022 12:43:07 +0200 Krzysztof wrote:
>
>>>>> There are sleep in atomic context bugs when the request to secure
>>>>> element of st21nfca is timeout. The root cause is that kzalloc and
>>>>> alloc_skb with GFP_KERNEL parameter is called in st21nfca_se_wt_timeout
>>>>> which is a timer handler. The call tree shows the execution paths that
>>>>> could lead to bugs:
>>>>>
>>>>> (Interrupt context)
>>>>> st21nfca_se_wt_timeout
>>>>> nfc_hci_send_event
>>>>> nfc_hci_hcp_message_tx
>>>>> kzalloc(..., GFP_KERNEL) //may sleep
>>>>> alloc_skb(..., GFP_KERNEL) //may sleep
>>>>>
>>>>> This patch changes allocation mode of kzalloc and alloc_skb from
>>>>> GFP_KERNEL to GFP_ATOMIC in order to prevent atomic context from
>>>>> sleeping. The GFP_ATOMIC flag makes memory allocation operation
>>>>> could be used in atomic context.
>>>>>
>>>>> Fixes: 8b8d2e08bf0d ("NFC: HCI support")
>>>>> Signed-off-by: Duoming Zhou <duoming@zju.edu.cn>
>>>>> ---
>>>>> net/nfc/hci/hcp.c | 4 ++--
>>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/net/nfc/hci/hcp.c b/net/nfc/hci/hcp.c
>>>>> index 05c60988f59..1caf9c2086f 100644
>>>>> --- a/net/nfc/hci/hcp.c
>>>>> +++ b/net/nfc/hci/hcp.c
>>>>> @@ -30,7 +30,7 @@ int nfc_hci_hcp_message_tx(struct nfc_hci_dev *hdev, u8 pipe,
>>>>> int hci_len, err;
>>>>> bool firstfrag = true;
>>>>>
>>>>> - cmd = kzalloc(sizeof(struct hci_msg), GFP_KERNEL);
>>>>> + cmd = kzalloc(sizeof(*cmd), GFP_ATOMIC);
>>>>
>>>> No, this does not look correct. This function can sleep, so it can use
>>>> GFP_KERNEL. Please just look at the function before replacing any flags...
>>>
>>> I am sorry, I don`t understand why nfc_hci_hcp_message_tx() can sleep.
>>
>> Why? because in line 93 it uses a mutex (which is a sleeping primitve).
>>
>>>
>>> I think st21nfca_se_wt_timeout() is a timer handler, which is in interrupt context.
>>> The call tree shows the execution paths that could lead to bugs:
>>>
>>> st21nfca_hci_se_io()
>>> mod_timer(&info->se_info.bwi_timer,...)
>>> st21nfca_se_wt_timeout() //timer handler, interrupt context
>>> nfc_hci_send_event()
>>> nfc_hci_hcp_message_tx()
>>> kzalloc(..., GFP_KERNEL) //may sleep
>>> alloc_skb(..., GFP_KERNEL) //may sleep
>>>
>>> What`s more, I think the "mutex_lock(&hdev->msg_tx_mutex)" called by nfc_hci_hcp_message_tx()
>>> is also improper.
>>>
>>> Please correct me, If you think I am wrong. Thanks for your time.
>>
>> Your patch is not correct in current semantics of this function. This
>> function can sleep (because it uses a mutex), so the mistake is rather
>> calling it from interrupt context.
>
> We have to call nfc_hci_send_event() in st21nfca_se_wt_timeout(), because we need to send
> a reset request as recovery procedure. I think replace GFP_KERNEL to GFP_ATOMIC and replace
> mutex_lock to spin_lock in nfc_hci_hcp_message_tx() is better.
>
> What's more, in order to synchronize with other functions related with hci message TX,
> We need to replace the mutex_lock(&hdev->msg_tx_mutex) to spin_lock in other functions
> as well. I sent "patch v2" just now.
You sent v2 one minute before replying here... that's not how discussion
work. Please do not sent next version before reaching some consensus.
Best regards,
Krzysztof
^ permalink raw reply
* [PATCH v2,bpf-next] samples/bpf: check detach prog exist or not in xdp_fwd
From: Zhengchao Shao @ 2022-05-17 11:27 UTC (permalink / raw)
To: bpf, netdev, linux-kernel, ast, daniel, davem, kuba, hawk,
john.fastabend, andrii, kafai, songliubraving, yhs, kpsingh
Cc: weiyongjun1, shaozhengchao, yuehaibing
Before detach the prog, we should check detach prog exist or not.
Signed-off-by: Zhengchao Shao <shaozhengchao@huawei.com>
---
samples/bpf/xdp_fwd_user.c | 52 +++++++++++++++++++++++++++++++-------
1 file changed, 43 insertions(+), 9 deletions(-)
diff --git a/samples/bpf/xdp_fwd_user.c b/samples/bpf/xdp_fwd_user.c
index 1828487bae9a..2294486ef10a 100644
--- a/samples/bpf/xdp_fwd_user.c
+++ b/samples/bpf/xdp_fwd_user.c
@@ -47,17 +47,51 @@ static int do_attach(int idx, int prog_fd, int map_fd, const char *name)
return err;
}
-static int do_detach(int idx, const char *name)
+static int do_detach(int idx, const char *name, const char *prog_name)
{
- int err;
+ int err = 1;
+ __u32 info_len, curr_prog_id;
+ struct bpf_prog_info prog_info = {};
+ int prog_fd;
+ char namepad[BPF_OBJ_NAME_LEN];
+
+ if (bpf_xdp_query_id(idx, xdp_flags, &curr_prog_id)) {
+ printf("ERROR: bpf_xdp_query_id failed\n");
+ return err;
+ }
+
+ if (!curr_prog_id) {
+ printf("ERROR: flags(0x%x) xdp prog is not attached to %s\n",
+ xdp_flags, name);
+ return err;
+ }
- err = bpf_xdp_detach(idx, xdp_flags, NULL);
- if (err < 0)
- printf("ERROR: failed to detach program from %s\n", name);
+ info_len = sizeof(prog_info);
+ prog_fd = bpf_prog_get_fd_by_id(curr_prog_id);
+ if (prog_fd < 0 && errno == ENOENT) {
+ printf("ERROR: bpf_prog_get_fd_by_id failed\n");
+ return err;
+ }
+
+ err = bpf_obj_get_info_by_fd(prog_fd, &prog_info, &info_len);
+ if (err) {
+ printf("ERROR: bpf_obj_get_info_by_fd failed\n");
+ return err;
+ }
+ snprintf(namepad, sizeof(namepad), "%s_prog", prog_name);
+
+ if (strcmp(prog_info.name, namepad)) {
+ printf("ERROR: %s isn't attached to %s\n", prog_name, name);
+ } else {
+ err = bpf_xdp_detach(idx, xdp_flags, NULL);
+ if (err < 0)
+ printf("ERROR: failed to detach program from %s\n",
+ name);
+ /* TODO: Remember to cleanup map, when adding use of shared map
+ * bpf_map_delete_elem((map_fd, &idx);
+ */
+ }
- /* TODO: Remember to cleanup map, when adding use of shared map
- * bpf_map_delete_elem((map_fd, &idx);
- */
return err;
}
@@ -169,7 +203,7 @@ int main(int argc, char **argv)
return 1;
}
if (!attach) {
- err = do_detach(idx, argv[i]);
+ err = do_detach(idx, argv[i], prog_name);
if (err)
ret = err;
} else {
--
2.17.1
^ permalink raw reply related
* Re: linux-next: build warning after merge of the net-next tree
From: Florian Westphal @ 2022-05-17 11:25 UTC (permalink / raw)
To: Stephen Rothwell
Cc: David Miller, Networking, Florian Westphal, Pablo Neira Ayuso,
Linux Kernel Mailing List, Linux Next Mailing List
In-Reply-To: <20220517190332.4506f7e8@canb.auug.org.au>
Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> Hi all,
>
> On Tue, 17 May 2022 11:03:03 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> >
> > After merging the net-next tree, today's linux-next build (powerpc
> > ppc64_defconfig) produced this warning:
> >
> > net/netfilter/nf_conntrack_netlink.c:1717:12: warning: 'ctnetlink_dump_one_entry' defined but not used [-Wunused-function]
> > 1717 | static int ctnetlink_dump_one_entry(struct sk_buff *skb,
> > | ^~~~~~~~~~~~~~~~~~~~~~~~
> >
> > Introduced by commit
> >
> > 8a75a2c17410 ("netfilter: conntrack: remove unconfirmed list")
>
> So for my i386 defconfig build this became on error, so I have applied
> the following patch for today.
>
> From: Stephen Rothwell <sfr@canb.auug.org.au>
> Date: Tue, 17 May 2022 18:58:43 +1000
> Subject: [PATCH] fix up for "netfilter: conntrack: remove unconfirmed list"
>
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
Thanks Stephen.
Acked-by: Florian Westphal <fw@strlen.de>
^ permalink raw reply
* Re: [PATCH v2] net: phy: marvell: Add errata section 5.1 for Alaska PHY
From: Stefan Roese @ 2022-05-17 11:21 UTC (permalink / raw)
To: Paolo Abeni, netdev
Cc: Leszek Polak, Marek Behún, Andrew Lunn, Heiner Kallweit,
Russell King, David S . Miller
In-Reply-To: <163e90e736803c670ce88f2b2b1174eddc1060a2.camel@redhat.com>
Hi Paolo,
On 17.05.22 13:01, Paolo Abeni wrote:
> Hello,
>
> On Mon, 2022-05-16 at 09:08 +0200, Stefan Roese wrote:
>> From: Leszek Polak <lpolak@arri.de>
>>
>> As per Errata Section 5.1, if EEE is intended to be used, some register
>> writes must be done once after every hardware reset. This patch now adds
>> the necessary register writes as listed in the Marvell errata.
>>
>> Without this fix we experience ethernet problems on some of our boards
>> equipped with a new version of this ethernet PHY (different supplier).
>>
>> The fix applies to Marvell Alaska 88E1510/88E1518/88E1512/88E1514
>> Rev. A0.
>>
>> Signed-off-by: Leszek Polak <lpolak@arri.de>
>> Signed-off-by: Stefan Roese <sr@denx.de>
>> Cc: Marek Behún <kabel@kernel.org>
>> Cc: Andrew Lunn <andrew@lunn.ch>
>> Cc: Heiner Kallweit <hkallweit1@gmail.com>
>> Cc: Russell King <linux@armlinux.org.uk>
>> Cc: David S. Miller <davem@davemloft.net>
>
> It's not clear to me if you are targeting -net or net-next, could you
> please clarify? In case this is for -net, please add a suitable fixes
> tag, thanks!
Sorry for not being clear on this. net-next is good AFAICT.
Should I re-submit to net-next?
Thanks,
Stefan
^ permalink raw reply
* Re: [PATCH] net: vxge: Remove unnecessary synchronize_irq() before free_irq()
From: patchwork-bot+netdevbpf @ 2022-05-17 11:20 UTC (permalink / raw)
To: CGEL
Cc: jdmason, davem, edumazet, kuba, pabeni, netdev, linux-kernel,
chi.minghao, zealci
In-Reply-To: <20220516081914.1651281-1-chi.minghao@zte.com.cn>
Hello:
This patch was applied to netdev/net-next.git (master)
by Paolo Abeni <pabeni@redhat.com>:
On Mon, 16 May 2022 08:19:14 +0000 you wrote:
> From: Minghao Chi <chi.minghao@zte.com.cn>
>
> Calling synchronize_irq() right before free_irq() is quite useless. On one
> hand the IRQ can easily fire again before free_irq() is entered, on the
> other hand free_irq() itself calls synchronize_irq() internally (in a race
> condition free way), before any state associated with the IRQ is freed.
>
> [...]
Here is the summary with links:
- net: vxge: Remove unnecessary synchronize_irq() before free_irq()
https://git.kernel.org/netdev/net-next/c/bd81bfb5a1d1
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH] qed: Remove unnecessary synchronize_irq() before free_irq()
From: patchwork-bot+netdevbpf @ 2022-05-17 11:20 UTC (permalink / raw)
To: CGEL
Cc: aelior, manishc, davem, edumazet, kuba, pabeni, netdev,
linux-kernel, chi.minghao, zealci
In-Reply-To: <20220516072646.1651109-1-chi.minghao@zte.com.cn>
Hello:
This patch was applied to netdev/net-next.git (master)
by Paolo Abeni <pabeni@redhat.com>:
On Mon, 16 May 2022 07:26:46 +0000 you wrote:
> From: Minghao Chi <chi.minghao@zte.com.cn>
>
> Calling synchronize_irq() right before free_irq() is quite useless. On one
> hand the IRQ can easily fire again before free_irq() is entered, on the
> other hand free_irq() itself calls synchronize_irq() internally (in a race
> condition free way), before any state associated with the IRQ is freed.
>
> [...]
Here is the summary with links:
- qed: Remove unnecessary synchronize_irq() before free_irq()
https://git.kernel.org/netdev/net-next/c/29fd3ca1779f
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH] net: qede: Remove unnecessary synchronize_irq() before free_irq()
From: patchwork-bot+netdevbpf @ 2022-05-17 11:20 UTC (permalink / raw)
To: CGEL
Cc: aelior, manishc, davem, edumazet, kuba, pabeni, netdev,
linux-kernel, chi.minghao, zealci
In-Reply-To: <20220516082251.1651350-1-chi.minghao@zte.com.cn>
Hello:
This patch was applied to netdev/net-next.git (master)
by Paolo Abeni <pabeni@redhat.com>:
On Mon, 16 May 2022 08:22:51 +0000 you wrote:
> From: Minghao Chi <chi.minghao@zte.com.cn>
>
> Calling synchronize_irq() right before free_irq() is quite useless. On one
> hand the IRQ can easily fire again before free_irq() is entered, on the
> other hand free_irq() itself calls synchronize_irq() internally (in a race
> condition free way), before any state associated with the IRQ is freed.
>
> [...]
Here is the summary with links:
- net: qede: Remove unnecessary synchronize_irq() before free_irq()
https://git.kernel.org/netdev/net-next/c/d1e7f009bfff
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* [PATCH v6 3/3] dt-bindings: usb: ci-hdrc-usb2: fix node node for ethernet controller
From: Oleksij Rempel @ 2022-05-17 11:15 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski
Cc: Oleksij Rempel, Rob Herring, kernel, netdev, linux-kernel,
devicetree
In-Reply-To: <20220517111505.929722-1-o.rempel@pengutronix.de>
This documentation provides wrong node name for the Ethernet controller.
It should be "ethernet" instead of "smsc" as required by Ethernet
controller devicetree schema:
Documentation/devicetree/bindings/net/ethernet-controller.yaml
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Acked-by: Rob Herring <robh@kernel.org>
---
Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
index a5c5db6a0b2d..ba51fb1252b9 100644
--- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
+++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
@@ -151,7 +151,7 @@ Example for HSIC:
#address-cells = <1>;
#size-cells = <0>;
- usbnet: smsc@1 {
+ usbnet: ethernet@1 {
compatible = "usb424,9730";
reg = <1>;
};
--
2.30.2
^ permalink raw reply related
* [PATCH v6 1/3] dt-bindings: net: add schema for ASIX USB Ethernet controllers
From: Oleksij Rempel @ 2022-05-17 11:15 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski
Cc: Oleksij Rempel, Rob Herring, kernel, netdev, linux-kernel,
devicetree
In-Reply-To: <20220517111505.929722-1-o.rempel@pengutronix.de>
Create schema for ASIX USB Ethernet controllers and import some of
currently supported USB IDs form drivers/net/usb/asix_devices.c
These devices are already used in some of DTs. So, this schema makes it official.
NOTE: there was no previously documented txt based DT binding for this
controllers.
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Reviewed-by: Rob Herring <robh@kernel.org>
---
.../devicetree/bindings/net/asix,ax88178.yaml | 68 +++++++++++++++++++
1 file changed, 68 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/asix,ax88178.yaml
diff --git a/Documentation/devicetree/bindings/net/asix,ax88178.yaml b/Documentation/devicetree/bindings/net/asix,ax88178.yaml
new file mode 100644
index 000000000000..1af52358de4c
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/asix,ax88178.yaml
@@ -0,0 +1,68 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/asix,ax88178.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: The device tree bindings for the USB Ethernet controllers
+
+maintainers:
+ - Oleksij Rempel <o.rempel@pengutronix.de>
+
+description: |
+ Device tree properties for hard wired USB Ethernet devices.
+
+allOf:
+ - $ref: ethernet-controller.yaml#
+
+properties:
+ compatible:
+ items:
+ - enum:
+ - usbb95,1720 # ASIX AX88172
+ - usbb95,172a # ASIX AX88172A
+ - usbb95,1780 # ASIX AX88178
+ - usbb95,7720 # ASIX AX88772
+ - usbb95,772a # ASIX AX88772A
+ - usbb95,772b # ASIX AX88772B
+ - usbb95,7e2b # ASIX AX88772B
+
+ reg: true
+ local-mac-address: true
+ mac-address: true
+
+required:
+ - compatible
+ - reg
+
+additionalProperties: false
+
+examples:
+ - |
+ usb {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ ethernet@1 {
+ compatible = "usbb95,7e2b";
+ reg = <1>;
+ local-mac-address = [00 00 00 00 00 00];
+ };
+ };
+ - |
+ usb {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ usb1@1 {
+ compatible = "usb1234,5678";
+ reg = <1>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ ethernet@1 {
+ compatible = "usbb95,772b";
+ reg = <1>;
+ };
+ };
+ };
--
2.30.2
^ permalink raw reply related
* [PATCH v6 2/3] dt-bindings: net: add schema for Microchip/SMSC LAN95xx USB Ethernet controllers
From: Oleksij Rempel @ 2022-05-17 11:15 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski
Cc: Oleksij Rempel, kernel, netdev, linux-kernel, devicetree
In-Reply-To: <20220517111505.929722-1-o.rempel@pengutronix.de>
Create initial schema for Microchip/SMSC LAN95xx USB Ethernet controllers and
import some of currently supported USB IDs form drivers/net/usb/smsc95xx.c
These devices are already used in some of DTs. So, this schema makes it official.
NOTE: there was no previously documented txt based DT binding for this
controllers.
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
.../bindings/net/microchip,lan95xx.yaml | 63 +++++++++++++++++++
1 file changed, 63 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/microchip,lan95xx.yaml
diff --git a/Documentation/devicetree/bindings/net/microchip,lan95xx.yaml b/Documentation/devicetree/bindings/net/microchip,lan95xx.yaml
new file mode 100644
index 000000000000..cf91fecd8909
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/microchip,lan95xx.yaml
@@ -0,0 +1,63 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/microchip,lan95xx.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: The device tree bindings for the USB Ethernet controllers
+
+maintainers:
+ - Oleksij Rempel <o.rempel@pengutronix.de>
+
+description: |
+ Device tree properties for hard wired SMSC95xx compatible USB Ethernet
+ controller.
+
+allOf:
+ - $ref: ethernet-controller.yaml#
+
+properties:
+ compatible:
+ items:
+ - enum:
+ - usb424,9500 # SMSC9500 USB Ethernet Device
+ - usb424,9505 # SMSC9505 USB Ethernet Device
+ - usb424,9530 # SMSC LAN9530 USB Ethernet Device
+ - usb424,9730 # SMSC LAN9730 USB Ethernet Device
+ - usb424,9900 # SMSC9500 USB Ethernet Device (SAL10)
+ - usb424,9901 # SMSC9505 USB Ethernet Device (SAL10)
+ - usb424,9902 # SMSC9500A USB Ethernet Device (SAL10)
+ - usb424,9903 # SMSC9505A USB Ethernet Device (SAL10)
+ - usb424,9904 # SMSC9512/9514 USB Hub & Ethernet Device (SAL10)
+ - usb424,9905 # SMSC9500A USB Ethernet Device (HAL)
+ - usb424,9906 # SMSC9505A USB Ethernet Device (HAL)
+ - usb424,9907 # SMSC9500 USB Ethernet Device (Alternate ID)
+ - usb424,9908 # SMSC9500A USB Ethernet Device (Alternate ID)
+ - usb424,9909 # SMSC9512/9514 USB Hub & Ethernet Devic. ID)
+ - usb424,9e00 # SMSC9500A USB Ethernet Device
+ - usb424,9e01 # SMSC9505A USB Ethernet Device
+ - usb424,9e08 # SMSC LAN89530 USB Ethernet Device
+ - usb424,ec00 # SMSC9512/9514 USB Hub & Ethernet Device
+
+ reg: true
+ local-mac-address: true
+ mac-address: true
+
+required:
+ - compatible
+ - reg
+
+additionalProperties: false
+
+examples:
+ - |
+ usb {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ ethernet@1 {
+ compatible = "usb424,9e00";
+ reg = <1>;
+ local-mac-address = [00 00 00 00 00 00];
+ };
+ };
--
2.30.2
^ permalink raw reply related
* [PATCH v6 0/3] document dt-schema for some USB Ethernet controllers
From: Oleksij Rempel @ 2022-05-17 11:15 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski
Cc: Oleksij Rempel, kernel, netdev, linux-kernel, devicetree
changes v6:
- remove USB hub example from microchip,lan95xx.yaml. We care only about
ethernet node.
- use only documented USD ID in example.
- add Reviewed-by
- drop board patches, all of them are taken by different subsystem
maintainers.
changes v5:
- move compatible string changes to a separate patch
- add note about possible regressions
changes v4:
- reword commit logs.
- add note about compatible fix
Oleksij Rempel (3):
dt-bindings: net: add schema for ASIX USB Ethernet controllers
dt-bindings: net: add schema for Microchip/SMSC LAN95xx USB Ethernet
controllers
dt-bindings: usb: ci-hdrc-usb2: fix node node for ethernet controller
.../devicetree/bindings/net/asix,ax88178.yaml | 68 +++++++++++++++++++
.../bindings/net/microchip,lan95xx.yaml | 63 +++++++++++++++++
.../devicetree/bindings/usb/ci-hdrc-usb2.txt | 2 +-
3 files changed, 132 insertions(+), 1 deletion(-)
create mode 100644 Documentation/devicetree/bindings/net/asix,ax88178.yaml
create mode 100644 Documentation/devicetree/bindings/net/microchip,lan95xx.yaml
--
2.30.2
^ permalink raw reply
* Re: [PATCH net-next v2] net: openvswitch: fix uAPI incompatibility with existing user space
From: Eelco Chaudron @ 2022-05-17 11:10 UTC (permalink / raw)
To: Vlad Buslov, Toms Atteka
Cc: Roi Dayan, Ilya Maximets, Aaron Conole, Jakub Kicinski,
David S. Miller, Pravin B Shelar, netdev, dev, linux-kernel,
Johannes Berg, Maor Dickman
In-Reply-To: <87lev783k8.fsf@nvidia.com>
On 12 May 2022, at 12:08, Vlad Buslov wrote:
> On Thu 12 May 2022 at 12:19, Eelco Chaudron <echaudro@redhat.com> wrote:
>> On 7 Apr 2022, at 12:22, Ilya Maximets wrote:
>>
>>> On 4/7/22 10:02, Vlad Buslov wrote:
>>>> On Mon 14 Mar 2022 at 20:40, Ilya Maximets <i.maximets@ovn.org> wrote:
>>>>> On 3/14/22 19:33, Roi Dayan wrote:
>>>>>>
>>>>>>
>>>>>> On 2022-03-10 8:44 PM, Aaron Conole wrote:
>>>>>>> Ilya Maximets <i.maximets@ovn.org> writes:
>>>>>>>
>>>>>>>> Few years ago OVS user space made a strange choice in the commit [1]
>>>>>>>> to define types only valid for the user space inside the copy of a
>>>>>>>> kernel uAPI header. '#ifndef __KERNEL__' and another attribute was
>>>>>>>> added later.
>>>>>>>>
>>>>>>>> This leads to the inevitable clash between user space and kernel types
>>>>>>>> when the kernel uAPI is extended. The issue was unveiled with the
>>>>>>>> addition of a new type for IPv6 extension header in kernel uAPI.
>>>>>>>>
>>>>>>>> When kernel provides the OVS_KEY_ATTR_IPV6_EXTHDRS attribute to the
>>>>>>>> older user space application, application tries to parse it as
>>>>>>>> OVS_KEY_ATTR_PACKET_TYPE and discards the whole netlink message as
>>>>>>>> malformed. Since OVS_KEY_ATTR_IPV6_EXTHDRS is supplied along with
>>>>>>>> every IPv6 packet that goes to the user space, IPv6 support is fully
>>>>>>>> broken.
>>>>>>>>
>>>>>>>> Fixing that by bringing these user space attributes to the kernel
>>>>>>>> uAPI to avoid the clash. Strictly speaking this is not the problem
>>>>>>>> of the kernel uAPI, but changing it is the only way to avoid breakage
>>>>>>>> of the older user space applications at this point.
>>>>>>>>
>>>>>>>> These 2 types are explicitly rejected now since they should not be
>>>>>>>> passed to the kernel. Additionally, OVS_KEY_ATTR_TUNNEL_INFO moved
>>>>>>>> out from the '#ifdef __KERNEL__' as there is no good reason to hide
>>>>>>>> it from the userspace. And it's also explicitly rejected now, because
>>>>>>>> it's for in-kernel use only.
>>>>>>>>
>>>>>>>> Comments with warnings were added to avoid the problem coming back.
>>>>>>>>
>>>>>>>> (1 << type) converted to (1ULL << type) to avoid integer overflow on
>>>>>>>> OVS_KEY_ATTR_IPV6_EXTHDRS, since it equals 32 now.
>>>>>>>>
>>>>>>>> [1] beb75a40fdc2 ("userspace: Switching of L3 packets in L2 pipeline")
>>>>>>>>
>>>>>>>> Fixes: 28a3f0601727 ("net: openvswitch: IPv6: Add IPv6 extension header support")
>>>>>>>> Link: https://lore.kernel.org/netdev/3adf00c7-fe65-3ef4-b6d7-6d8a0cad8a5f@nvidia.com
>>>>>>>> Link: https://github.com/openvswitch/ovs/commit/beb75a40fdc295bfd6521b0068b4cd12f6de507c
>>>>>>>> Reported-by: Roi Dayan <roid@nvidia.com>
>>>>>>>> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
>>>>>>>> ---
>>>>>>>
>>>>>>> Acked-by: Aaron Conole <aconole@redhat.com>
>>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> I got to check traffic with the fix and I do get some traffic
>>>>>> but something is broken. I didn't investigate much but the quick
>>>>>> test shows me rules are not offloaded and dumping ovs rules gives
>>>>>> error like this
>>>>>>
>>>>>> recirc_id(0),in_port(enp8s0f0_1),ct_state(-trk),eth(),eth_type(0x86dd),ipv6(frag=no)(bad
>>>>>> key length 2, expected -1)(00 00/(bad mask length 2, expected -1)(00 00),
>>>>>> packets:2453, bytes:211594, used:0.004s, flags:S., actions:ct,recirc(0x2)
>>>>>
>>>>> Such a dump is expected, because kernel parses fields that current
>>>>> userspace doesn't understand, and at the same time OVS by design is
>>>>> using kernel provided key/mask while installing datapath rules, IIRC.
>>>>> It should be possible to make these dumps a bit more friendly though.
>>>>>
>>>>> For the offloading not working, see my comment in the v2 patch email
>>>>> I sent (top email of this thread). In short, it's a problem in user
>>>>> space and it can not be fixed from the kernel side, unless we revert
>>>>> IPv6 extension header support and never add any new types, which is
>>>>> unreasonable. I didn't test any actual offloading, but I had a
>>>>> successful run of 'make check-offloads' with my quick'n'dirty fix from
>>>>> the top email.
>>>>
>>>> Hi Ilya,
>>>>
>>>> I can confirm that with latest OvS master IPv6 rules offload still fails
>>>> without your pastebin code applied.
>>>>
>>>>>
>>>>> Since we're here:
>>>>>
>>>>> Toms, do you plan to submit user space patches for this feature?
>>>>
>>>> I see there is a patch from you that is supposed to fix compatibility
>>>> issues caused by this change in OvS d96d14b14733 ("openvswitch.h: Align
>>>> uAPI definition with the kernel."), but it doesn't fix offload for me
>>>> without pastebin patch.
>>>
>>> Yes. OVS commit d96d14b14733 is intended to only fix the uAPI.
>>> Issue with offload is an OVS bug that should be fixed separately.
>>> The fix will also need to be backported to OVS stable branches.
>>>
>>>> Do you plan to merge that code into OvS or you
>>>> require some help from our side?
>>>
>>> I could do that, but I don't really have enough time. So, if you
>>> can work on that fix, it would be great. Note that comments inside
>>> the OVS's lib/odp-util.c:parse_key_and_mask_to_match() was blindly
>>> copied from the userspace datapath and are incorrect for the general
>>> case, so has to be fixed alongside the logic of that function.
>>
>> Tom or Vlad, are you working on this? Asking, as the release of a kernel with
>> Tom’s “net: openvswitch: IPv6: Add IPv6 extension header support” patch will
>> break OVS.
>>
>> //Eelco
>
> Hi Eelco,
>
> My simple fix for OvS was rejected and I don't have time to rework it at
> the moment.
That’s a pity, Tom do you maybe have time as your patch left OVS in this error state?
//Eelco
^ 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