* Re: [PATCH AUTOSEL 4.9 08/12] can: sja1000: Add Quirk for RZ/N1 SJA1000 CAN controller
From: Pavel Machek @ 2022-08-13 13:44 UTC (permalink / raw)
To: Sasha Levin
Cc: linux-kernel, stable, Biju Das, Marc Kleine-Budde, wg, davem,
edumazet, kuba, pabeni, mailhol.vincent, stefan.maetje, socketcan,
linux-can, netdev
In-Reply-To: <20220811161144.1543598-8-sashal@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 551 bytes --]
Hi!
> As per Chapter 6.5.16 of the RZ/N1 Peripheral Manual, The SJA1000
> CAN controller does not support Clock Divider Register compared to
> the reference Philips SJA1000 device.
>
> This patch adds a device quirk to handle this difference.
I don't think this is suitable for stable (at least 5.10.X and older)
as we don't have user of the quirk queued up.
Best regards,
Pavel
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply
* Re: [PATCH AUTOSEL 4.9 05/12] mlxsw: cmd: Increase 'config_profile.flood_mode' length
From: Pavel Machek @ 2022-08-13 13:43 UTC (permalink / raw)
To: Sasha Levin
Cc: linux-kernel, stable, Amit Cohen, Ido Schimmel, David S . Miller,
petrm, edumazet, kuba, pabeni, netdev
In-Reply-To: <20220811161144.1543598-5-sashal@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 663 bytes --]
Hi!
> From: Amit Cohen <amcohen@nvidia.com>
>
> [ Upstream commit 89df3c6261f271c550f120b5ccf4d9c5132e870c ]
>
> Currently, the length of 'config_profile.flood_mode' is defined as 2
> bits, while the correct length is 3 bits.
>
> As preparation for unified bridge model, which will use the whole field
> length, fix it and increase the field to the correct size.
I don't think we need here as follow-up patches are not queued for at
least 4.9 and 4.19...
Best regards,
Pavel
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply
* [PATCH] net: fix potential refcount leak in ndisc_router_discovery()
From: Xin Xiong @ 2022-08-13 12:49 UTC (permalink / raw)
To: David S. Miller, Hideaki YOSHIFUJI, David Ahern, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Praveen Chaudhary, Zhenggen Xu,
netdev, linux-kernel
Cc: yuanxzhang, Xin Xiong, Xin Tan
The issue happens on specific paths in the function. After both the
object `rt` and `neigh` are grabbed successfully, when `lifetime` is
nonzero but the metric needs change, the function just deletes the
route and set `rt` to NULL. Then, it may try grabbing `rt` and `neigh`
again if above conditions hold. The function simply overwrite `neigh`
if succeeds or returns if fails, without decreasing the reference
count of previous `neigh`. This may result in memory leaks.
Fix it by decrementing the reference count of `neigh` in place.
Fixes: 6b2e04bc240f ("net: allow user to set metric on default route learned via Router Advertisement")
Signed-off-by: Xin Xiong <xiongx18@fudan.edu.cn>
Signed-off-by: Xin Tan <tanxin.ctf@gmail.com>
---
net/ipv6/ndisc.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index 98453693e400..3a553494ff16 100644
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -1378,6 +1378,9 @@ static void ndisc_router_discovery(struct sk_buff *skb)
if (!rt && lifetime) {
ND_PRINTK(3, info, "RA: adding default router\n");
+ if (neigh)
+ neigh_release(neigh);
+
rt = rt6_add_dflt_router(net, &ipv6_hdr(skb)->saddr,
skb->dev, pref, defrtr_usr_metric);
if (!rt) {
--
2.25.1
^ permalink raw reply related
* Re: [RFC net-next v3 23/29] io_uring: allow to pass addr into sendzc
From: Stefan Metzmacher @ 2022-08-13 8:45 UTC (permalink / raw)
To: Pavel Begunkov, io-uring, netdev, linux-kernel
Cc: David S . Miller, Jakub Kicinski, Jonathan Lemon,
Willem de Bruijn, Jens Axboe, kernel-team
In-Reply-To: <56631a36-fec8-9c41-712b-195ad7e4cb9f@gmail.com>
Hi Pavel,
>> Given that this fills in msg almost completely can we also have
>> a version of SENDMSGZC, it would be very useful to also allow
>> msg_control to be passed and as well as an iovec.
>>
>> Would that be possible?
>
> Right, I left it to follow ups as the series is already too long.
>
> fwiw, I'm going to also add addr to IORING_OP_SEND.
Given the minimal differences, which were left between
IORING_OP_SENDZC and IORING_OP_SEND, wouldn't it be better
to merge things to IORING_OP_SEND using a IORING_RECVSEND_ZC_NOTIF
as indication to use the notif slot.
It would means we don't need to waste two opcodes for
IORING_OP_SENDZC and IORING_OP_SENDMSGZC (and maybe more)
I also noticed a problem in io_notif_update()
for (; idx < idx_end; idx++) {
struct io_notif_slot *slot = &ctx->notif_slots[idx];
if (!slot->notif)
continue;
if (up->arg)
slot->tag = up->arg;
io_notif_slot_flush_submit(slot, issue_flags);
}
slot->tag = up->arg is skipped if there is no notif already.
So you can't just use a 2 linked sqe's with
IORING_RSRC_UPDATE_NOTIF followed by IORING_OP_SENDZC(with IORING_RECVSEND_NOTIF_FLUSH)
I think the if (!slot->notif) should be moved down a bit.
It would somehow be nice to avoid the notif slots at all and somehow
use some kind of multishot request in order to generate two qces.
I'm also wondering what will happen if a notif will be referenced by the net layer
but the io_uring instance is already closed, wouldn't
io_uring_tx_zerocopy_callback() or __io_notif_complete_tw() crash
because notif->ctx is a stale pointer, of notif itself is already gone...
What do you think?
metze
^ permalink raw reply
* Re: [PATCH v2 1/1] net: qrtr: start MHI channel after endpoit creation
From: Loic Poulain @ 2022-08-13 8:45 UTC (permalink / raw)
To: Jakub Kicinski, Maxim Kochetkov
Cc: netdev, davem, edumazet, pabeni, linux-arm-msm, Hemant Kumar,
Manivannan Sadhasivam
In-Reply-To: <20220812170926.0370b05d@kernel.org>
On Sat, 13 Aug 2022 at 02:09, Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Thu, 11 Aug 2022 12:48:40 +0300 Maxim Kochetkov wrote:
> > MHI channel may generates event/interrupt right after enabling.
> > It may leads to 2 race conditions issues.
> >
> > 1)
> > Such event may be dropped by qcom_mhi_qrtr_dl_callback() at check:
> >
> > if (!qdev || mhi_res->transaction_status)
> > return;
> >
> > Because dev_set_drvdata(&mhi_dev->dev, qdev) may be not performed at
> > this moment. In this situation qrtr-ns will be unable to enumerate
> > services in device.
> > ---------------------------------------------------------------
> >
> > 2)
> > Such event may come at the moment after dev_set_drvdata() and
> > before qrtr_endpoint_register(). In this case kernel will panic with
> > accessing wrong pointer at qcom_mhi_qrtr_dl_callback():
> >
> > rc = qrtr_endpoint_post(&qdev->ep, mhi_res->buf_addr,
> > mhi_res->bytes_xferd);
> >
> > Because endpoint is not created yet.
> > --------------------------------------------------------------
> > So move mhi_prepare_for_transfer_autoqueue after endpoint creation
> > to fix it.
> >
> > Fixes: a2e2cc0dbb11 ("net: qrtr: Start MHI channels during init")
> > Signed-off-by: Maxim Kochetkov <fido_max@inbox.ru>
> > Reviewed-by: Hemant Kumar <quic_hemantk@quicinc.com>
> > Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
Reviewed-by: Loic Poulain <loic.poulain@linaro.org>
>
> You must CC the author of the patch under Fixes, they are usually
> the best person to review the fix. Adding Loic now.
>
> > diff --git a/net/qrtr/mhi.c b/net/qrtr/mhi.c
> > index 18196e1c8c2f..9ced13c0627a 100644
> > --- a/net/qrtr/mhi.c
> > +++ b/net/qrtr/mhi.c
> > @@ -78,11 +78,6 @@ static int qcom_mhi_qrtr_probe(struct mhi_device *mhi_dev,
> > struct qrtr_mhi_dev *qdev;
> > int rc;
> >
> > - /* start channels */
> > - rc = mhi_prepare_for_transfer_autoqueue(mhi_dev);
> > - if (rc)
> > - return rc;
> > -
> > qdev = devm_kzalloc(&mhi_dev->dev, sizeof(*qdev), GFP_KERNEL);
> > if (!qdev)
> > return -ENOMEM;
> > @@ -96,6 +91,13 @@ static int qcom_mhi_qrtr_probe(struct mhi_device *mhi_dev,
> > if (rc)
> > return rc;
> >
> > + /* start channels */
> > + rc = mhi_prepare_for_transfer_autoqueue(mhi_dev);
> > + if (rc) {
> > + qrtr_endpoint_unregister(&qdev->ep);
> > + return rc;
> > + }
> > +
> > dev_dbg(qdev->dev, "Qualcomm MHI QRTR driver probed\n");
> >
> > return 0;
>
^ permalink raw reply
* Re: Query on reads being flagged as direct writes...
From: Maciej Żenczykowski @ 2022-08-13 7:52 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: Lina Wang, Linux NetDev, BPF Mailing List, Jesper Dangaard Brouer,
Thomas Graf, Alexei Starovoitov
In-Reply-To: <CAKH8qBuiGU91htP5C4N_zCeRVSF9cgPFy7gh55YMA29sbtJHhw@mail.gmail.com>
I haven't tried figuring it out yet (via printks)... as I can't
currently trigger this myself, so I'm basically stuck with code
spelunking.
(I do know we currently legitimately do at least one dpa write... and
converting that one line to use bpf_skb_store_bytes results in the
program not even loading...
https://android-review.googlesource.com/c/platform/packages/modules/Connectivity/+/2181376
- but I haven't yet had the opportunity to figure out what, likely
obvious, mistake I made)
We do make use of at least the following helpers:
bpf_map_lookup_elem
bpf_map_update_elem
which AFAICT are all marked as pkt_access == true, even though we
don't use them to read nor write to the packet.
Having said that, and having dug deeper into the code I think only
may_access_direct_pkt_data(env, meta, BPF_READ) is the problematic
call site, and AFAICT it always has meta != NULL since it is called
via check_func_arg(env, i, &meta, fn).
So maybe this does just work? even if it is super confusing... and
should probably be documented better.
ie. right now we have two callers of may_access_direct_pkt_data():
may_access_direct_pkt_data(env, NULL, BPF_WRITE)
may_access_direct_pkt_data(env, non-NULL, BPF_READ)
so meta != NULL implies t == BPF_WRITE, and using fallthrough would be
a no-op (with current callers)
Maybe this is just a single function that does two very different
things in the two call sites...
On Fri, Aug 12, 2022 at 9:58 AM Stanislav Fomichev <sdf@google.com> wrote:
>
> On Fri, Aug 12, 2022 at 5:06 AM Maciej Żenczykowski
> <zenczykowski@gmail.com> wrote:
> >
> > From kernel/bpf/verifier.c with some simplifications (removed some of
> > the cases to make this shorter):
> >
> > static bool may_access_direct_pkt_data(struct bpf_verifier_env *env,
> > const struct bpf_call_arg_meta *meta, enum bpf_access_type t)
> > {
> > enum bpf_prog_type prog_type = resolve_prog_type(env->prog);
> > switch (prog_type) {
> > /* Program types only with direct read access go here! */
> > case BPF_PROG_TYPE_CGROUP_SKB: (and some others)
> > if (t == BPF_WRITE) return false;
> > fallthrough;
> > /* Program types with direct read + write access go here! */
> > case BPF_PROG_TYPE_SCHED_CLS: (and some others)
> > if (meta) return meta->pkt_access;
> > env->seen_direct_write = true;
> > return true;
> > case BPF_PROG_TYPE_CGROUP_SOCKOPT:
> > if (t == BPF_WRITE) env->seen_direct_write = true;
> > return true;
> > }
> > }
> >
> > why does the above set env->seen_direct_write to true even when t !=
> > BPF_WRITE, even for programs that only allow (per the comment) direct
> > read access.
> >
> > Is this working correctly? Is there some gotcha this is papering over?
> >
> > Should 'env->seen_direct_write = true; return true;' be changed into
> > 'fallthrough' so that write is only set if t == BPF_WRITE?
> >
> > This matters because 'env->seen_direct_write = true' then triggers an
> > unconditional unclone in the bpf prologue, which I'd like to avoid
> > unless I actually need to modify the packet (with
> > bpf_skb_store_bytes)...
> >
> > may_access_direct_pkt_data() has two call sites, in one it only gets
> > called with BPF_WRITE so it's ok, but the other one is in
> > check_func_arg():
> >
> > if (type_is_pkt_pointer(type) && !may_access_direct_pkt_data(env,
> > meta, BPF_READ)) { verbose(env, "helper access to the packet is not
> > allowed\n"); return -EACCES; }
> >
> > and I'm not really following what this does, but it seems like bpf
> > helper read access to the packet triggers unclone?
>
> There seems to be a set of helpers (pkt_access=true) which accept
> direct packet pointers and are known to be doing only reads of the skb
> data (safe without clone).
> You seem to be hitting the case where you're passing that packet
> pointer to one of the "unsafe" (pkt_acces=false) helpers which
> triggers that seen_direct_write=true condition.
> So it seems like it's by design? Which helper are you calling? Maybe
> that one should also have pkt_access=true?
>
> Tangential: I wish there was an explicit BPF_F_MAY_ATTEMPT_TO_CLONE
> flag that gates this auto-clone. I think at some point we also
> accidentally hit it :-(
>
> > (side note: all packets ingressing from the rndis gadget driver are
> > clones due to how it deals with usb packet deaggregation [not to be
> > mistaken with lro/tso])
> >
> > Confused...
^ permalink raw reply
* [PATCH] wireguard: send/receive: update function names in comments
From: Yuntao Wang @ 2022-08-13 4:35 UTC (permalink / raw)
To: Jason A. Donenfeld
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
wireguard, netdev, linux-kernel, Yuntao Wang
The functions packet_send_queued_handshakes() and
packet_process_queued_handshake_packets() were renamed to
wg_packet_handshake_send_worker() and wg_packet_handshake_receive_worker()
respectively, but the comments referring to them were not updated
accordingly, let's fix it.
Signed-off-by: Yuntao Wang <ytcoode@gmail.com>
---
drivers/net/wireguard/receive.c | 2 +-
drivers/net/wireguard/send.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireguard/receive.c b/drivers/net/wireguard/receive.c
index 7135d51d2d87..5b9cd1841390 100644
--- a/drivers/net/wireguard/receive.c
+++ b/drivers/net/wireguard/receive.c
@@ -566,7 +566,7 @@ void wg_packet_receive(struct wg_device *wg, struct sk_buff *skb)
}
atomic_inc(&wg->handshake_queue_len);
cpu = wg_cpumask_next_online(&wg->handshake_queue.last_cpu);
- /* Queues up a call to packet_process_queued_handshake_packets(skb): */
+ /* Queues up a call to wg_packet_handshake_receive_worker(skb): */
queue_work_on(cpu, wg->handshake_receive_wq,
&per_cpu_ptr(wg->handshake_queue.worker, cpu)->work);
break;
diff --git a/drivers/net/wireguard/send.c b/drivers/net/wireguard/send.c
index 5368f7c35b4b..15202c2e91a8 100644
--- a/drivers/net/wireguard/send.c
+++ b/drivers/net/wireguard/send.c
@@ -69,8 +69,8 @@ void wg_packet_send_queued_handshake_initiation(struct wg_peer *peer,
goto out;
wg_peer_get(peer);
- /* Queues up calling packet_send_queued_handshakes(peer), where we do a
- * peer_put(peer) after:
+ /* Queues up calling wg_packet_handshake_send_worker(peer), where we do
+ * a wg_peer_put(peer) after:
*/
if (!queue_work(peer->device->handshake_send_wq,
&peer->transmit_handshake_work))
--
2.37.1
^ permalink raw reply related
* Re: [PATCH net-next v2] net: skb: prevent the split of kfree_skb_reason() by gcc
From: kernel test robot @ 2022-08-13 2:09 UTC (permalink / raw)
To: menglong8.dong, kuba, miguel.ojeda.sandonis
Cc: llvm, kbuild-all, ojeda, ndesaulniers, davem, edumazet, pabeni,
asml.silence, imagedong, luiz.von.dentz, vasily.averin, jk,
linux-kernel, netdev
In-Reply-To: <20220812025015.316609-1-imagedong@tencent.com>
Hi,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
url: https://github.com/intel-lab-lkp/linux/commits/menglong8-dong-gmail-com/net-skb-prevent-the-split-of-kfree_skb_reason-by-gcc/20220812-105214
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 7ebfc85e2cd7b08f518b526173e9a33b56b3913b
config: i386-randconfig-a002 (https://download.01.org/0day-ci/archive/20220813/202208131003.M7FBGBna-lkp@intel.com/config)
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 5f1c7e2cc5a3c07cbc2412e851a7283c1841f520)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/ec98116fd4b985103e65c71d47f9390fee025cb9
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review menglong8-dong-gmail-com/net-skb-prevent-the-split-of-kfree_skb_reason-by-gcc/20220812-105214
git checkout ec98116fd4b985103e65c71d47f9390fee025cb9
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash net/core/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> net/core/skbuff.c:780:6: warning: unknown attribute '__optimize__' ignored [-Wunknown-attributes]
void __nofnsplit
^~~~~~~~~~~
include/linux/compiler_attributes.h:273:56: note: expanded from macro '__nofnsplit'
#define __nofnsplit __attribute__((__optimize__("O1")))
^~~~~~~~~~~~~~~~~~
1 warning generated.
vim +/__optimize__ +780 net/core/skbuff.c
770
771 /**
772 * kfree_skb_reason - free an sk_buff with special reason
773 * @skb: buffer to free
774 * @reason: reason why this skb is dropped
775 *
776 * Drop a reference to the buffer and free it if the usage count has
777 * hit zero. Meanwhile, pass the drop reason to 'kfree_skb'
778 * tracepoint.
779 */
> 780 void __nofnsplit
781 kfree_skb_reason(struct sk_buff *skb, enum skb_drop_reason reason)
782 {
783 if (!skb_unref(skb))
784 return;
785
786 DEBUG_NET_WARN_ON_ONCE(reason <= 0 || reason >= SKB_DROP_REASON_MAX);
787
788 trace_kfree_skb(skb, __builtin_return_address(0), reason);
789 __kfree_skb(skb);
790 }
791 EXPORT_SYMBOL(kfree_skb_reason);
792
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply
* Re: [PATCH net 1/3] netfilter: nf_conntrack_tcp: re-init for syn packets only
From: Thomas Backlund @ 2022-08-13 1:26 UTC (permalink / raw)
To: Jakub Kicinski, Neal Cardwell, stable
Cc: patchwork-bot+netdevbpf, Pablo Neira Ayuso, netfilter-devel,
davem, netdev, Yuchung Cheng, Eric Dumazet
Den 2022-08-12 kl. 22:17, skrev Jakub Kicinski:
> On Fri, 12 Aug 2022 09:34:14 -0400 Neal Cardwell wrote:
>> This first commit is an important bug fix for a serious bug that causes
>> TCP connection hangs for users of TCP fast open and nf_conntrack:
>>
>> c7aab4f17021b netfilter: nf_conntrack_tcp: re-init for syn packets only
>>
>> We are continuing to get reports about the bug that this commit fixes.
>>
>> It seems this fix was only backported to v5.17 stable release, and not further,
>> due to a cherry-pick conflict, because this fix implicitly depends on a
>> slightly earlier v5.17 fix in the same spot:
>>
>> 82b72cb94666 netfilter: conntrack: re-init state for retransmitted syn-ack
>>
>> I manually verified that the fix c7aab4f17021b can be cleanly cherry-picked
>> into the oldest (v4.9.325) and newest (v5.15.60) longterm release kernels as
>> long as we first cherry-pick that related fix that it implicitly depends on:
>>
>> 82b72cb94666b3dbd7152bb9f441b068af7a921b
>> netfilter: conntrack: re-init state for retransmitted syn-ack
>>
>> c7aab4f17021b636a0ee75bcf28e06fb7c94ab48
>> netfilter: nf_conntrack_tcp: re-init for syn packets only
>>
>> So would it be possible to backport both of those fixes with the following
>> cherry-picks, to all LTS stable releases?
>>
>> git cherry-pick 82b72cb94666b3dbd7152bb9f441b068af7a921b
>> git cherry-pick c7aab4f17021b636a0ee75bcf28e06fb7c94ab48
>
> Thanks a lot Neal! FWIW we have recently changed our process and no
> longer handle stable submissions ourselves, so in the future feel free
> to talk directly to stable@ (and add CC: stable@ tags to patches).
>
> I'm adding stable@, let's see if Greg & team can pick things up based
> on your instructions :)
>
besides testing that they apply,
one should also check that the resulting code actually builds...
net/netfilter/nf_conntrack_proto_tcp.c: In function 'tcp_in_window':
net/netfilter/nf_conntrack_proto_tcp.c:560:3: error: implicit
declaration of function 'tcp_init_sender'; did you mean 'tcp_init_cwnd'?
[-Werror=implicit-function-declaration]
So this one is also needed:
cc4f9d62037ebcb811f4908bba2986c01df1bd50
netfilter: conntrack: move synack init code to helper
for it to actually build on 5.15
--
Thomas
^ permalink raw reply
* Re: [PATCH v2 net-next] net: ethernet: ti: davinci_mdio: Add workaround for errata i2329
From: kernel test robot @ 2022-08-13 0:57 UTC (permalink / raw)
To: Ravi Gunasekaran, davem
Cc: llvm, kbuild-all, andrew, edumazet, kuba, pabeni, linux-omap,
netdev, linux-kernel, linux-arm-kernel, kishon, vigneshr,
r-gunasekaran
In-Reply-To: <20220810111345.31200-1-r-gunasekaran@ti.com>
Hi Ravi,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on net-next/master]
url: https://github.com/intel-lab-lkp/linux/commits/Ravi-Gunasekaran/net-ethernet-ti-davinci_mdio-Add-workaround-for-errata-i2329/20220810-191718
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git f86d1fbbe7858884d6754534a0afbb74fc30bc26
config: arm-randconfig-r026-20220810 (https://download.01.org/0day-ci/archive/20220813/202208130814.216FrNfX-lkp@intel.com/config)
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 5f1c7e2cc5a3c07cbc2412e851a7283c1841f520)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install arm cross compiling tool for clang build
# apt-get install binutils-arm-linux-gnueabi
# https://github.com/intel-lab-lkp/linux/commit/c62c93111418d5468f6add98d244f0a594dbe352
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Ravi-Gunasekaran/net-ethernet-ti-davinci_mdio-Add-workaround-for-errata-i2329/20220810-191718
git checkout c62c93111418d5468f6add98d244f0a594dbe352
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash drivers/net/ethernet/ti/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> drivers/net/ethernet/ti/davinci_mdio.c:548:37: error: use of undeclared identifier 'k3_mdio_socinfo'
soc_match_data = soc_device_match(k3_mdio_socinfo);
^
>> drivers/net/ethernet/ti/davinci_mdio.c:553:31: error: incomplete definition of type 'struct k3_mdio_soc_data'
data->manual_mode = socdata->manual_mode;
~~~~~~~^
drivers/net/ethernet/ti/davinci_mdio.c:550:17: note: forward declaration of 'struct k3_mdio_soc_data'
const struct k3_mdio_soc_data *socdata =
^
2 errors generated.
vim +/k3_mdio_socinfo +548 drivers/net/ethernet/ti/davinci_mdio.c
528
529 static int davinci_mdio_probe(struct platform_device *pdev)
530 {
531 struct mdio_platform_data *pdata = dev_get_platdata(&pdev->dev);
532 struct device *dev = &pdev->dev;
533 struct davinci_mdio_data *data;
534 struct resource *res;
535 struct phy_device *phy;
536 int ret, addr;
537 int autosuspend_delay_ms = -1;
538 const struct soc_device_attribute *soc_match_data;
539
540 data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
541 if (!data)
542 return -ENOMEM;
543
544 data->manual_mode = false;
545 data->bb_ctrl.ops = &davinci_mdiobb_ops;
546
547 if (IS_ENABLED(CONFIG_OF) && dev->of_node) {
> 548 soc_match_data = soc_device_match(k3_mdio_socinfo);
549 if (soc_match_data && soc_match_data->data) {
550 const struct k3_mdio_soc_data *socdata =
551 soc_match_data->data;
552
> 553 data->manual_mode = socdata->manual_mode;
554 }
555 }
556
557 if (data->manual_mode)
558 data->bus = alloc_mdio_bitbang(&data->bb_ctrl);
559 else
560 data->bus = devm_mdiobus_alloc(dev);
561
562 if (!data->bus) {
563 dev_err(dev, "failed to alloc mii bus\n");
564 return -ENOMEM;
565 }
566
567 if (IS_ENABLED(CONFIG_OF) && dev->of_node) {
568 const struct davinci_mdio_of_param *of_mdio_data;
569
570 ret = davinci_mdio_probe_dt(&data->pdata, pdev);
571 if (ret)
572 return ret;
573 snprintf(data->bus->id, MII_BUS_ID_SIZE, "%s", pdev->name);
574
575 of_mdio_data = of_device_get_match_data(&pdev->dev);
576 if (of_mdio_data) {
577 autosuspend_delay_ms =
578 of_mdio_data->autosuspend_delay_ms;
579 }
580 } else {
581 data->pdata = pdata ? (*pdata) : default_pdata;
582 snprintf(data->bus->id, MII_BUS_ID_SIZE, "%s-%x",
583 pdev->name, pdev->id);
584 }
585
586 data->bus->name = dev_name(dev);
587
588 if (data->manual_mode) {
589 data->bus->read = davinci_mdiobb_read;
590 data->bus->write = davinci_mdiobb_write;
591 data->bus->reset = davinci_mdiobb_reset;
592
593 dev_info(dev, "Configuring MDIO in manual mode\n");
594 } else {
595 data->bus->read = davinci_mdio_read;
596 data->bus->write = davinci_mdio_write;
597 data->bus->reset = davinci_mdio_reset;
598 data->bus->priv = data;
599 }
600 data->bus->parent = dev;
601
602 data->clk = devm_clk_get(dev, "fck");
603 if (IS_ERR(data->clk)) {
604 dev_err(dev, "failed to get device clock\n");
605 return PTR_ERR(data->clk);
606 }
607
608 dev_set_drvdata(dev, data);
609 data->dev = dev;
610
611 res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
612 if (!res)
613 return -EINVAL;
614 data->regs = devm_ioremap(dev, res->start, resource_size(res));
615 if (!data->regs)
616 return -ENOMEM;
617
618 davinci_mdio_init_clk(data);
619
620 pm_runtime_set_autosuspend_delay(&pdev->dev, autosuspend_delay_ms);
621 pm_runtime_use_autosuspend(&pdev->dev);
622 pm_runtime_enable(&pdev->dev);
623
624 /* register the mii bus
625 * Create PHYs from DT only in case if PHY child nodes are explicitly
626 * defined to support backward compatibility with DTs which assume that
627 * Davinci MDIO will always scan the bus for PHYs detection.
628 */
629 if (dev->of_node && of_get_child_count(dev->of_node))
630 data->skip_scan = true;
631
632 ret = of_mdiobus_register(data->bus, dev->of_node);
633 if (ret)
634 goto bail_out;
635
636 /* scan and dump the bus */
637 for (addr = 0; addr < PHY_MAX_ADDR; addr++) {
638 phy = mdiobus_get_phy(data->bus, addr);
639 if (phy) {
640 dev_info(dev, "phy[%d]: device %s, driver %s\n",
641 phy->mdio.addr, phydev_name(phy),
642 phy->drv ? phy->drv->name : "unknown");
643 }
644 }
645
646 return 0;
647
648 bail_out:
649 pm_runtime_dont_use_autosuspend(&pdev->dev);
650 pm_runtime_disable(&pdev->dev);
651 return ret;
652 }
653
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply
* Re: [PATCH] net: dsa: mv88e6060: prevent crash on an unused port
From: patchwork-bot+netdevbpf @ 2022-08-13 0:50 UTC (permalink / raw)
To: Sergei Antonov; +Cc: netdev, vivien.didelot, f.fainelli, davem, olteanv
In-Reply-To: <20220811070939.1717146-1-saproj@gmail.com>
Hello:
This patch was applied to netdev/net.git (master)
by Jakub Kicinski <kuba@kernel.org>:
On Thu, 11 Aug 2022 10:09:39 +0300 you wrote:
> If the port isn't a CPU port nor a user port, 'cpu_dp'
> is a null pointer and a crash happened on dereferencing
> it in mv88e6060_setup_port():
>
> [ 9.575872] Unable to handle kernel NULL pointer dereference at virtual address 00000014
> ...
> [ 9.942216] mv88e6060_setup from dsa_register_switch+0x814/0xe84
> [ 9.948616] dsa_register_switch from mdio_probe+0x2c/0x54
> [ 9.954433] mdio_probe from really_probe.part.0+0x98/0x2a0
> [ 9.960375] really_probe.part.0 from driver_probe_device+0x30/0x10c
> [ 9.967029] driver_probe_device from __device_attach_driver+0xb8/0x13c
> [ 9.973946] __device_attach_driver from bus_for_each_drv+0x90/0xe0
> [ 9.980509] bus_for_each_drv from __device_attach+0x110/0x184
> [ 9.986632] __device_attach from bus_probe_device+0x8c/0x94
> [ 9.992577] bus_probe_device from deferred_probe_work_func+0x78/0xa8
> [ 9.999311] deferred_probe_work_func from process_one_work+0x290/0x73c
> [ 10.006292] process_one_work from worker_thread+0x30/0x4b8
> [ 10.012155] worker_thread from kthread+0xd4/0x10c
> [ 10.017238] kthread from ret_from_fork+0x14/0x3c
>
> [...]
Here is the summary with links:
- net: dsa: mv88e6060: prevent crash on an unused port
https://git.kernel.org/netdev/net/c/246bbf2f977e
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] fec: Fix timer capture timing in `fec_ptp_enable_pps()`
From: patchwork-bot+netdevbpf @ 2022-08-13 0:50 UTC (permalink / raw)
To: =?utf-8?b?Q3PDs2vDoXMgQmVuY2UgPGNzb2thcy5iZW5jZUBwcm9sYW4uaHU+?=
Cc: netdev, richardcochran
In-Reply-To: <20220811101348.13755-1-csokas.bence@prolan.hu>
Hello:
This patch was applied to netdev/net.git (master)
by Jakub Kicinski <kuba@kernel.org>:
On Thu, 11 Aug 2022 12:13:49 +0200 you wrote:
> Code reimplements functionality already in `fec_ptp_read()`,
> but misses check for FEC_QUIRK_BUG_CAPTURE. Replace with function call.
>
> Signed-off-by: Csókás Bence <csokas.bence@prolan.hu>
> ---
> drivers/net/ethernet/freescale/fec_ptp.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
Here is the summary with links:
- fec: Fix timer capture timing in `fec_ptp_enable_pps()`
https://git.kernel.org/netdev/net/c/61d5e2a251fb
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 2/2] ice: Fix call trace with null VSI during VF reset
From: Jakub Kicinski @ 2022-08-13 0:13 UTC (permalink / raw)
To: Tony Nguyen
Cc: davem, pabeni, edumazet, Michal Jaron, netdev, Jedrzej Jagielski,
Marek Szlosek
In-Reply-To: <20220811161714.305094-3-anthony.l.nguyen@intel.com>
On Thu, 11 Aug 2022 09:17:14 -0700 Tony Nguyen wrote:
> This WARN_ON() is unnecessary and causes call trace, despite that
> call trace, driver still works. There is no need for this warn
> because this piece of code is responsible for disabling VF's Tx/Rx
> queues when VF is disabled, but when VF is already removed there
> is no need to do reset or disable queues.
Can't you flush the service work when disabling VFs instead?
Seems better to try to keep the system in a consistent state
than add "if NULL return;" in random places :S
^ permalink raw reply
* [PATCHv2 bpf-next] libbpf: making bpf_prog_load() ignore name if kernel doesn't support
From: Hangbin Liu @ 2022-08-13 0:09 UTC (permalink / raw)
To: netdev
Cc: Quentin Monnet, Andrii Nakryiko, Alexei Starovoitov,
Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
bpf, Hangbin Liu
Similar with commit 10b62d6a38f7 ("libbpf: Add names for auxiliary maps"),
let's make bpf_prog_load() also ignore name if kernel doesn't support
program name.
To achieve this, we need to call sys_bpf_prog_load() directly in
probe_kern_prog_name() to avoid circular dependency. sys_bpf_prog_load()
also need to be exported in the libbpf_internal.h file.
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
v2: move sys_bpf_prog_load definition to libbpf_internal.h. memset attr
to 0 specifically to aviod padding.
---
tools/lib/bpf/bpf.c | 6 ++----
tools/lib/bpf/libbpf.c | 12 ++++++++++--
tools/lib/bpf/libbpf_internal.h | 3 +++
3 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
index 6a96e665dc5d..575867d69496 100644
--- a/tools/lib/bpf/bpf.c
+++ b/tools/lib/bpf/bpf.c
@@ -84,9 +84,7 @@ static inline int sys_bpf_fd(enum bpf_cmd cmd, union bpf_attr *attr,
return ensure_good_fd(fd);
}
-#define PROG_LOAD_ATTEMPTS 5
-
-static inline int sys_bpf_prog_load(union bpf_attr *attr, unsigned int size, int attempts)
+int sys_bpf_prog_load(union bpf_attr *attr, unsigned int size, int attempts)
{
int fd;
@@ -263,7 +261,7 @@ int bpf_prog_load(enum bpf_prog_type prog_type,
attr.prog_ifindex = OPTS_GET(opts, prog_ifindex, 0);
attr.kern_version = OPTS_GET(opts, kern_version, 0);
- if (prog_name)
+ if (prog_name && kernel_supports(NULL, FEAT_PROG_NAME))
libbpf_strlcpy(attr.prog_name, prog_name, sizeof(attr.prog_name));
attr.license = ptr_to_u64(license);
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 3f01f5cd8a4c..4a351897bdcc 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -4419,10 +4419,18 @@ static int probe_kern_prog_name(void)
BPF_MOV64_IMM(BPF_REG_0, 0),
BPF_EXIT_INSN(),
};
- int ret, insn_cnt = ARRAY_SIZE(insns);
+ union bpf_attr attr;
+ int ret;
+
+ memset(&attr, 0, sizeof(attr));
+ attr.prog_type = BPF_PROG_TYPE_SOCKET_FILTER;
+ attr.license = ptr_to_u64("GPL");
+ attr.insns = ptr_to_u64(insns);
+ attr.insn_cnt = (__u32)ARRAY_SIZE(insns);
+ libbpf_strlcpy(attr.prog_name, "test", sizeof(attr.prog_name));
/* make sure loading with name works */
- ret = bpf_prog_load(BPF_PROG_TYPE_SOCKET_FILTER, "test", "GPL", insns, insn_cnt, NULL);
+ ret = sys_bpf_prog_load(&attr, sizeof(attr), PROG_LOAD_ATTEMPTS);
return probe_fd(ret);
}
diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
index 4135ae0a2bc3..377642ff51fc 100644
--- a/tools/lib/bpf/libbpf_internal.h
+++ b/tools/lib/bpf/libbpf_internal.h
@@ -573,4 +573,7 @@ static inline bool is_pow_of_2(size_t x)
return x && (x & (x - 1)) == 0;
}
+#define PROG_LOAD_ATTEMPTS 5
+int sys_bpf_prog_load(union bpf_attr *attr, unsigned int size, int attempts);
+
#endif /* __LIBBPF_LIBBPF_INTERNAL_H */
--
2.31.1
^ permalink raw reply related
* Re: [PATCH v2 1/1] net: qrtr: start MHI channel after endpoit creation
From: Jakub Kicinski @ 2022-08-13 0:09 UTC (permalink / raw)
To: Maxim Kochetkov, loic.poulain
Cc: netdev, davem, edumazet, pabeni, linux-arm-msm, Hemant Kumar,
Manivannan Sadhasivam
In-Reply-To: <20220811094840.1654088-1-fido_max@inbox.ru>
On Thu, 11 Aug 2022 12:48:40 +0300 Maxim Kochetkov wrote:
> MHI channel may generates event/interrupt right after enabling.
> It may leads to 2 race conditions issues.
>
> 1)
> Such event may be dropped by qcom_mhi_qrtr_dl_callback() at check:
>
> if (!qdev || mhi_res->transaction_status)
> return;
>
> Because dev_set_drvdata(&mhi_dev->dev, qdev) may be not performed at
> this moment. In this situation qrtr-ns will be unable to enumerate
> services in device.
> ---------------------------------------------------------------
>
> 2)
> Such event may come at the moment after dev_set_drvdata() and
> before qrtr_endpoint_register(). In this case kernel will panic with
> accessing wrong pointer at qcom_mhi_qrtr_dl_callback():
>
> rc = qrtr_endpoint_post(&qdev->ep, mhi_res->buf_addr,
> mhi_res->bytes_xferd);
>
> Because endpoint is not created yet.
> --------------------------------------------------------------
> So move mhi_prepare_for_transfer_autoqueue after endpoint creation
> to fix it.
>
> Fixes: a2e2cc0dbb11 ("net: qrtr: Start MHI channels during init")
> Signed-off-by: Maxim Kochetkov <fido_max@inbox.ru>
> Reviewed-by: Hemant Kumar <quic_hemantk@quicinc.com>
> Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
You must CC the author of the patch under Fixes, they are usually
the best person to review the fix. Adding Loic now.
> diff --git a/net/qrtr/mhi.c b/net/qrtr/mhi.c
> index 18196e1c8c2f..9ced13c0627a 100644
> --- a/net/qrtr/mhi.c
> +++ b/net/qrtr/mhi.c
> @@ -78,11 +78,6 @@ static int qcom_mhi_qrtr_probe(struct mhi_device *mhi_dev,
> struct qrtr_mhi_dev *qdev;
> int rc;
>
> - /* start channels */
> - rc = mhi_prepare_for_transfer_autoqueue(mhi_dev);
> - if (rc)
> - return rc;
> -
> qdev = devm_kzalloc(&mhi_dev->dev, sizeof(*qdev), GFP_KERNEL);
> if (!qdev)
> return -ENOMEM;
> @@ -96,6 +91,13 @@ static int qcom_mhi_qrtr_probe(struct mhi_device *mhi_dev,
> if (rc)
> return rc;
>
> + /* start channels */
> + rc = mhi_prepare_for_transfer_autoqueue(mhi_dev);
> + if (rc) {
> + qrtr_endpoint_unregister(&qdev->ep);
> + return rc;
> + }
> +
> dev_dbg(qdev->dev, "Qualcomm MHI QRTR driver probed\n");
>
> return 0;
^ permalink raw reply
* Re: [PATCH bpf-next 13/15] mm, memcg: Add new helper get_obj_cgroup_from_cgroup
From: Yafang Shao @ 2022-08-13 0:07 UTC (permalink / raw)
To: Shakeel Butt
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin Lau,
Song Liu, Yonghong Song, john fastabend, KP Singh,
Stanislav Fomichev, Hao Luo, jolsa, Johannes Weiner, Michal Hocko,
Roman Gushchin, Muchun Song, Andrew Morton, netdev, bpf, Linux MM
In-Reply-To: <20220812165756.dxaqy3go567prr5s@google.com>
On Sat, Aug 13, 2022 at 12:57 AM Shakeel Butt <shakeelb@google.com> wrote:
>
> On Wed, Aug 10, 2022 at 03:18:38PM +0000, Yafang Shao wrote:
> > Introduce new helper get_obj_cgroup_from_cgroup() to get obj_cgroup from
> > a specific cgroup.
>
> Can you please add couple of lines on why you need objcg?
>
Sure. will update in the next version.
> >
> > Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> > ---
> > include/linux/memcontrol.h | 1 +
> > mm/memcontrol.c | 41 +++++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 42 insertions(+)
> >
> > diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> > index 2f0a611..901a921 100644
> > --- a/include/linux/memcontrol.h
> > +++ b/include/linux/memcontrol.h
> > @@ -1713,6 +1713,7 @@ static inline void set_shrinker_bit(struct mem_cgroup *memcg,
> > int __memcg_kmem_charge_page(struct page *page, gfp_t gfp, int order);
> > void __memcg_kmem_uncharge_page(struct page *page, int order);
> >
> > +struct obj_cgroup *get_obj_cgroup_from_cgroup(struct cgroup *cgrp);
> > struct obj_cgroup *get_obj_cgroup_from_current(void);
> > struct obj_cgroup *get_obj_cgroup_from_page(struct page *page);
> >
> > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> > index 618c366..762cffa 100644
> > --- a/mm/memcontrol.c
> > +++ b/mm/memcontrol.c
> > @@ -2908,6 +2908,47 @@ static struct obj_cgroup *__get_obj_cgroup_from_memcg(struct mem_cgroup *memcg)
> > return objcg;
> > }
> >
> > +static struct obj_cgroup *get_obj_cgroup_from_memcg(struct mem_cgroup *memcg)
> > +{
> > + struct obj_cgroup *objcg;
> > +
> > + if (memcg_kmem_bypass())
> > + return NULL;
> > +
> > + rcu_read_lock();
> > + objcg = __get_obj_cgroup_from_memcg(memcg);
> > + rcu_read_unlock();
> > + return objcg;
> > +}
> > +
> > +struct obj_cgroup *get_obj_cgroup_from_cgroup(struct cgroup *cgrp)
> > +{
> > + struct cgroup_subsys_state *css;
> > + struct mem_cgroup *memcg;
> > + struct obj_cgroup *objcg;
> > +
> > + rcu_read_lock();
> > + css = rcu_dereference(cgrp->subsys[memory_cgrp_id]);
> > + if (!css || !css_tryget_online(css)) {
> > + rcu_read_unlock();
> > + cgroup_put(cgrp);
> > + return ERR_PTR(-EINVAL);
> > + }
> > + rcu_read_unlock();
> > + cgroup_put(cgrp);
>
> The above put seems out of place and buggy.
>
Thanks for pointing it out.
The cgroup_put should be used in bpf_map_save_memcg().
I will update it.
--
Regards
Yafang
^ permalink raw reply
* Re: [WIP v2] igc: fix deadlock caused by taking RTNL in RPM resume path
From: Vinicius Costa Gomes @ 2022-08-13 0:05 UTC (permalink / raw)
To: James Hogan
Cc: Paul Menzel, Tony Nguyen, Jesse Brandeburg, netdev,
intel-wired-lan, Sasha Neftin, Aleksandr Loktionov
In-Reply-To: <4759452.31r3eYUQgx@saruman>
Hi James,
James Hogan <jhogan@kernel.org> writes:
> On Thursday, 11 August 2022 21:25:24 BST Vinicius Costa Gomes wrote:
>> It was reported a RTNL deadlock in the igc driver that was causing
>> problems during suspend/resume.
>>
>> The solution is similar to commit ac8c58f5b535 ("igb: fix deadlock
>> caused by taking RTNL in RPM resume path").
>>
>> Reported-by: James Hogan <jhogan@kernel.org>
>> Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
>> ---
>> Sorry for the noise earlier, my kernel config didn't have runtime PM
>> enabled.
>
> Thanks for looking into this.
>
> This is identical to the patch I've been running for the last week. The
> deadlock is avoided, however I now occasionally see an assertion from
> netif_set_real_num_tx_queues due to the lock not being taken in some cases via
> the runtime_resume path, and a suspicious rcu_dereference_protected() warning
> (presumably due to the same issue of the lock not being taken). See here for
> details:
> https://lore.kernel.org/netdev/4765029.31r3eYUQgx@saruman/
Oh, sorry. I missed the part that the rtnl assert splat was already
using similar/identical code to what I got/copied from igb.
So what this seems to be telling us is that the "fix" from igb is only
hiding the issue, and we would need to remove the need for taking the
RTNL for the suspend/resume paths in igc and igb? (as someone else said
in that igb thread, iirc)
Cheers,
--
Vinicius
^ permalink raw reply
* Re: [PATCH] net: fec: Restart PPS after link state change
From: Jakub Kicinski @ 2022-08-12 23:59 UTC (permalink / raw)
To: Csókás Bence
Cc: netdev, Richard Cochran, David S. Miller, qiangqing.zhang
In-Reply-To: <20220812115408.12985-1-csokas.bence@prolan.hu>
On Fri, 12 Aug 2022 13:54:09 +0200 Csókás Bence wrote:
> On link state change, the controller gets reset,
> causing PPS to drop out and the PHC to lose its
> time and calibration. So we restart it if needed,
> restoring calibration and time registers.
>
> Changes since v2:
> * Add `fec_ptp_save_state()`/`fec_ptp_restore_state()`
> * Use `ktime_get_real_ns()`
> * Use `BIT()` macro
> Changes since v1:
> * More ECR #define's
> * Stop PPS in `fec_ptp_stop()`
nit: [PATCH net v3] net: fec: ...
would have been the perfect prefix here.
The patch doesn't seem to apply, it needs to go to this tree:
https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/
When you repost, please also CC Andrew since he was giving feedback
on the initial version.
^ permalink raw reply
* Re: [PATCH bpf-next 13/15] mm, memcg: Add new helper get_obj_cgroup_from_cgroup
From: Yafang Shao @ 2022-08-12 23:56 UTC (permalink / raw)
To: Roman Gushchin
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin Lau,
Song Liu, Yonghong Song, john fastabend, KP Singh,
Stanislav Fomichev, Hao Luo, jolsa, Johannes Weiner, Michal Hocko,
Shakeel Butt, Muchun Song, Andrew Morton, netdev, bpf, Linux MM
In-Reply-To: <YvaQhLk06MHQJWHB@P9FQF9L96D.corp.robot.car>
On Sat, Aug 13, 2022 at 1:40 AM Roman Gushchin <roman.gushchin@linux.dev> wrote:
>
> On Fri, Aug 12, 2022 at 08:35:19AM +0800, Yafang Shao wrote:
> > On Fri, Aug 12, 2022 at 12:16 AM Roman Gushchin
> > <roman.gushchin@linux.dev> wrote:
> > >
> > > On Wed, Aug 10, 2022 at 03:18:38PM +0000, Yafang Shao wrote:
> > > > Introduce new helper get_obj_cgroup_from_cgroup() to get obj_cgroup from
> > > > a specific cgroup.
> > > >
> > > > Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> > > > ---
> > > > include/linux/memcontrol.h | 1 +
> > > > mm/memcontrol.c | 41 +++++++++++++++++++++++++++++++++++++++++
> > > > 2 files changed, 42 insertions(+)
> > > >
> > > > diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> > > > index 2f0a611..901a921 100644
> > > > --- a/include/linux/memcontrol.h
> > > > +++ b/include/linux/memcontrol.h
> > > > @@ -1713,6 +1713,7 @@ static inline void set_shrinker_bit(struct mem_cgroup *memcg,
> > > > int __memcg_kmem_charge_page(struct page *page, gfp_t gfp, int order);
> > > > void __memcg_kmem_uncharge_page(struct page *page, int order);
> > > >
> > > > +struct obj_cgroup *get_obj_cgroup_from_cgroup(struct cgroup *cgrp);
> > > > struct obj_cgroup *get_obj_cgroup_from_current(void);
> > > > struct obj_cgroup *get_obj_cgroup_from_page(struct page *page);
> > > >
> > > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> > > > index 618c366..762cffa 100644
> > > > --- a/mm/memcontrol.c
> > > > +++ b/mm/memcontrol.c
> > > > @@ -2908,6 +2908,47 @@ static struct obj_cgroup *__get_obj_cgroup_from_memcg(struct mem_cgroup *memcg)
> > > > return objcg;
> > > > }
> > > >
> > > > +static struct obj_cgroup *get_obj_cgroup_from_memcg(struct mem_cgroup *memcg)
> > > > +{
> > > > + struct obj_cgroup *objcg;
> > > > +
> > > > + if (memcg_kmem_bypass())
> > > > + return NULL;
> > > > +
> > > > + rcu_read_lock();
> > > > + objcg = __get_obj_cgroup_from_memcg(memcg);
> > > > + rcu_read_unlock();
> > > > + return objcg;
> > >
> > > This code doesn't make sense to me. What does rcu read lock protect here?
> >
> > To protect rcu_dereference(memcg->objcg);.
> > Doesn't it need the read rcu lock ?
>
> No, it's not how rcu works. Please, take a look at the docs here:
> https://docs.kernel.org/RCU/whatisRCU.html#whatisrcu .
> In particular, it describes this specific case very well.
>
> In 2 words, you don't protect the rcu_dereference() call, you protect the pointer
I just copied and pasted rcu_dereference(memcg->objcg) there to make it clear.
Actually it protects memcg->objcg, doesn't it ?
> you get, cause it's valid only inside the rcu read section. After rcu_read_unlock()
> it might point at a random data, because the protected object can be already freed.
>
Are you sure?
Can't the obj_cgroup_tryget(objcg) prevent it from being freed ?
--
Regards
Yafang
^ permalink raw reply
* Re: [Patch net v2 3/4] tcp: refactor tcp_read_skb() a bit
From: Jakub Kicinski @ 2022-08-12 23:55 UTC (permalink / raw)
To: Cong Wang
Cc: netdev, bpf, Cong Wang, Eric Dumazet, John Fastabend,
Jakub Sitnicki
In-Reply-To: <20220808033106.130263-4-xiyou.wangcong@gmail.com>
On Sun, 7 Aug 2022 20:31:05 -0700 Cong Wang wrote:
> From: Cong Wang <cong.wang@bytedance.com>
>
> As tcp_read_skb() only reads one skb at a time, the while loop is
> unnecessary, we can turn it into an if. This also simplifies the
> code logic.
I think Eric is AFK so we should just apply these, they LGTM.
One minor nit below.
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -1761,27 +1761,18 @@ int tcp_read_skb(struct sock *sk, skb_read_actor_t recv_actor)
> if (sk->sk_state == TCP_LISTEN)
> return -ENOTCONN;
>
> - while ((skb = tcp_recv_skb(sk, seq, &offset)) != NULL) {
> - int used;
> -
> + skb = tcp_recv_skb(sk, seq, &offset);
> + if (skb) {
if (!skb)
return 0;
?
^ permalink raw reply
* Re: [PATCH bpf-next] libbpf: making bpf_prog_load() ignore name if kernel doesn't support
From: Hangbin Liu @ 2022-08-12 23:53 UTC (permalink / raw)
To: Quentin Monnet
Cc: netdev, Andrii Nakryiko, Alexei Starovoitov, Daniel Borkmann,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, bpf
In-Reply-To: <407e67b7-b4f2-40db-6e13-409784fe32aa@isovalent.com>
On Fri, Aug 12, 2022 at 4:38 PM Quentin Monnet <quentin@isovalent.com> wrote:
> > diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
> > index 9c50beabdd14..125c580e45f8 100644
> > --- a/tools/lib/bpf/bpf.h
> > +++ b/tools/lib/bpf/bpf.h
> > @@ -35,6 +35,9 @@
> > extern "C" {
> > #endif
> >
> > +#define PROG_LOAD_ATTEMPTS 5
> > +int sys_bpf_prog_load(union bpf_attr *attr, unsigned int size, int attempts);
> > +
>
> bpf.h is the user-facing header, should these go into libbpf_internal.h
> instead?
libbpf_internal.h makes more sense, I wil move it.
> > + union bpf_attr attr = {
> > + .prog_type = BPF_PROG_TYPE_SOCKET_FILTER,
> > + .prog_name = "test",
> > + .license = ptr_to_u64("GPL"),
> > + .insns = ptr_to_u64(insns),
> > + .insn_cnt = (__u32)ARRAY_SIZE(insns),
> > + };
>
> I think you cannot initialise "attr" directly, you need a "memset(&attr,
> 0, sizeof(attr));" first, in case the struct contains padding between
> the fields.
Thanks for the reminder. I forgot the padding issue.
I will post version 2.
Thanks
Hangbin
^ permalink raw reply
* Re: [RFC net-next 3/4] ynl: add a sample python library
From: Jakub Kicinski @ 2022-08-12 23:07 UTC (permalink / raw)
To: Edward Cree
Cc: Stephen Hemminger, netdev, davem, edumazet, pabeni, sdf,
jacob.e.keller, vadfed, johannes, jiri, dsahern, fw, linux-doc
In-Reply-To: <0e27c04a-f17c-7491-7482-46dc9a5dd151@gmail.com>
On Fri, 12 Aug 2022 16:42:53 +0100 Edward Cree wrote:
> > It would be great if python had standard module for netlink.
> > Then your code could just (re)use that.
> > Something like mnl but for python.
>
> There's pyroute2, that seemed alright when I used it for something
> a few years back, and I think it has the pieces you need.
> https://pyroute2.org/
I saw that and assumed that its true to its name and only supports
RTNL :( I'll reach out to Peter the maintainer for his thoughts.
This patch was meant as a sample, I kept trying to finish up the C
codegen for a week and it still didn't feel presentable enough for
the RFC. In practice I'd rather leave writing the language specific
libs to the experts.
^ permalink raw reply
* Re: [RFC net-next 3/4] ynl: add a sample python library
From: Jakub Kicinski @ 2022-08-12 22:53 UTC (permalink / raw)
To: Stephen Hemminger
Cc: netdev, davem, edumazet, pabeni, sdf, jacob.e.keller, vadfed,
johannes, jiri, dsahern, fw, linux-doc
In-Reply-To: <20220811130906.198b091d@hermes.local>
On Thu, 11 Aug 2022 13:09:06 -0700 Stephen Hemminger wrote:
> Looks interesting, you might want to consider running your code
> through some of the existing Python checkers such as flake8 and pylint.
> If you want this to be generally available in repos, best to follow the language conventions
>
> For example flake8 noticed:
> $ flake8 --max-line-length=120 ./tools/net/ynl/samples/ynl.py
> ./tools/net/ynl/samples/ynl.py:251:55: F821 undefined name 'file_name'
Thanks! I'll make sure to check flake8 (pylint is too noisy for me :()
^ permalink raw reply
* Re: [RFC net-next 4/4] ynl: add a sample user for ethtool
From: Jakub Kicinski @ 2022-08-12 22:48 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: netdev, davem, edumazet, pabeni, jacob.e.keller, vadfed, johannes,
jiri, dsahern, stephen, fw, linux-doc
In-Reply-To: <CAKH8qBuzHMMG8T3mD5mZmY0N1Tit+yp1H-EQebmUsutAma9yCw@mail.gmail.com>
On Fri, 12 Aug 2022 09:26:13 -0700 Stanislav Fomichev wrote:
> On Thu, Aug 11, 2022 at 4:31 PM Jakub Kicinski <kuba@kernel.org> wrote:
> > Let me think about it some more. My main motivation is people writing
> > new families, I haven't sent too much time worrying about the existing
> > ones with all their quirks. It's entirely possible that the uAPI quirks
> > can just go and we won't generate uAPI for existing families as it
> > doesn't buy us anything.
>
> Ack. Although, we have to have some existing examples for people to
> write new families. So you might still have to convert something :-)
We do seem to have at least a couple on the horizon for 6.1.
Another thought I had yesterday was that maybe we want to have two
schemas - one "full" with all the historical baggage. And one "blessed"
for new families which narrows the options?
> > Still not convinced about messages, as it makes me think that every
> > "space" is then a definition of a message rather than just container
> > for field definitions with independent ID spaces.
> >
> > Attribute-sets sounds good, happy to rename.
> >
> > Another thought I just had was to call it something like "data-types"
> > or "field-types" or "type-spaces". To indicate the split into "data"
> > and "actions"/"operations"?
>
> I like attribute-set better than attribute-space :-)
OK!
> > Herm, looking at my commits where I started going with the C codegen
> > (which I haven't posted here) is converting the values to the same
> > format as keys (i.e. YAML/JSON style with dashes). So the codegen does:
> >
> > c_name = attr['name']
> > if c_name in c_keywords:
> > c_name += '_'
> > c_name = c_name.replace('-', '_')
> >
> > So the name would be "dev-index", C will make that dev_index, Java will
> > make that devIndex (or whatever) etc.
> >
> > I really don't want people to have to prefix the names because that's
> > creating more work. We can slap a /* header.dev_index */ comment in
> > the generated uAPI, for the grep? Dunno..
>
> Yeah, dunno as well, not sure how much of the per-language knowledge
> you should bake into the tool itself..
By "the tool" you mean the codegen? I assumed that's gotta be pretty
language specific. TBH I expected everyone will write their own codegen
or support library. Like I don't think Rust people will want to look at
my nasty Python C generator :S
I'd prefer to keep as much C-ness out of the spec as possible but
some may have to leak in because of the need to gen the uAPI.
> I think my confusion mostly
> comes from the fact that 'name' is mixed together with 'name-prefix'
> and one is 'low_caps' while the other one is 'ALL_CAPS'. Too much
> magic :-)
Fair point, at the very least they should be consistent, I'll fix.
> Thinking out loud: maybe these transformations should all go via some
> extra/separate yaml (or separate sections)? Like:
>
> fixup-attribute-sets:
> - name: header
> canonical-c-name: "ETHTOOL_A_HEADER_{name.upper()}"
> - name: channels
> canonical-c-name: "ETHTOOL_A_CHANNELS_{name.upper()}"
>
> fixup-operations:
> canonical-c-name: "ETHTOOL_MSG_{name.upper()}"
>
> # in case the "generic" catch-all above doesn't work
> - name: channgels_get
> canonical-c-name: "ETHTOOL_MSG_CHANNELS_GET"
>
> We can call it "compatibility" yaml and put all sorts of weird stuff in it.
> New users hopefully don't need to care about it and don't need to
> write any of the -prefix/-suffix stuff.
Let me chew on this for a little bit.
> > Why does Python need to know the length of the string tho?
> > On receive if kernel gives you a longer name - great, no problem.
> > On send the kernel will tell you so also meh.
>
> I was thinking that you wanted to do some client size validation as
> well? As in sizeof(rx_buf) > ALTIFNAMSIZ -> panic? But I agree that
> there is really no value in that, the kernel will do the validation
> anyway..
Yup, I don't see the point, the ext_ack handling must be solid anyway,
so the kernel error is as good as local panic for cases which should
"never happen" (famous last words, maybe, we'll see)
^ permalink raw reply
* Re: [RFC net-next 0/4] ynl: YAML netlink protocol descriptions
From: Jakub Kicinski @ 2022-08-12 22:26 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev, davem, edumazet, pabeni, sdf, jacob.e.keller, vadfed,
johannes, jiri, dsahern, stephen, fw, linux-doc
In-Reply-To: <9208fec1-60e9-dd2b-af27-ada3dfa50121@gmail.com>
On Fri, 12 Aug 2022 10:00:52 -0700 Florian Fainelli wrote:
> > The ability for a high level language like Python to talk to the kernel
> > so easily, without ctypes, manually packing structs, copy'n'pasting
> > values for defines etc. excites me more than C codegen, anyway.
>
> This is really cool BTW, and it makes a lot of sense to me that we are
> moving that way, especially with Rust knocking at the door. I will try
> to do a more thorough review, than "cool, I like it".
Thanks! Feel free to ping me for the latest version whenever you want
to take a look, because the code will hopefully be under very active
development for a few more weeks..
^ 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