* Re: [PATCH net v4] net: ti: icssg-prueth: Add missing icss_iep_put to error path
From: Anwar, Md Danish @ 2023-11-10 16:18 UTC (permalink / raw)
To: Jan Kiszka, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, MD Danish Anwar
Cc: netdev, linux-kernel, Lopes Ivo, Diogo Miguel (T CED IFD-PT),
Nishanth Menon, Su, Bao Cheng (RC-CN DF FA R&D),
Wojciech Drewek, Roger Quadros
In-Reply-To: <7a4e5c5b-e397-479b-b1cb-4b50da248f21@siemens.com>
On 11/10/2023 9:43 PM, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
>
> Analogously to prueth_remove, just also taking care for NULL'ing the
> iep pointers.
>
> Fixes: 186734c15886 ("net: ti: icssg-prueth: add packet timestamping and ptp support")
> Fixes: 443a2367ba3c ("net: ti: icssg-prueth: am65x SR2.0 add 10M full duplex support")
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>
Reviewed-by: MD Danish Anwar <danishanwar@ti.com>
> ---
>
> Changes in v4:
> - no functional ones
> - added one original author in CC with new address (no address of
> Grygorii available)
>
> Changes in v3:
> - consolidate cleanup logic further [Wojciech]
> - make sure to NULL iep pointers
>
> Changes in v2:
> - add proper tags
>
> This was lost from the TI SDK version while ripping out SR1.0 support -
> which we are currently restoring for upstream.
>
> drivers/net/ethernet/ti/icssg/icssg_prueth.c | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.c b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
> index 6c4b64227ac8..3abbeba26f1b 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.c
> +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
> @@ -2105,10 +2105,7 @@ static int prueth_probe(struct platform_device *pdev)
> prueth->iep1 = icss_iep_get_idx(np, 1);
> if (IS_ERR(prueth->iep1)) {
> ret = dev_err_probe(dev, PTR_ERR(prueth->iep1), "iep1 get failed\n");
> - icss_iep_put(prueth->iep0);
> - prueth->iep0 = NULL;
> - prueth->iep1 = NULL;
> - goto free_pool;
> + goto put_iep0;
> }
>
> if (prueth->pdata.quirk_10m_link_issue) {
> @@ -2205,6 +2202,12 @@ static int prueth_probe(struct platform_device *pdev)
> exit_iep:
> if (prueth->pdata.quirk_10m_link_issue)
> icss_iep_exit_fw(prueth->iep1);
> + icss_iep_put(prueth->iep1);
> +
> +put_iep0:
> + icss_iep_put(prueth->iep0);
> + prueth->iep0 = NULL;
> + prueth->iep1 = NULL;
>
> free_pool:
> gen_pool_free(prueth->sram_pool,
--
Thanks and Regards,
Md Danish Anwar
^ permalink raw reply
* Re: [PATCH] MAINTAINERS: net: Update reviewers for TI's Ethernet drivers
From: Anwar, Md Danish @ 2023-11-10 16:21 UTC (permalink / raw)
To: Ravi Gunasekaran, Roger Quadros, netdev
Cc: linux-omap, linux-kernel, s-vadapalli, nm, srk, Md Danish Anwar
In-Reply-To: <44f68604-b37d-56d9-6fc1-4c4cc503abd3@ti.com>
On 11/10/2023 3:08 PM, Ravi Gunasekaran wrote:
> Roger,
>
> On 11/10/23 2:21 PM, Roger Quadros wrote:
>> Hi Ravi,
>>
>> On 10/11/2023 10:42, Ravi Gunasekaran wrote:
>>> Grygorii is no longer associated with TI and messages addressed to
>>> him bounce.
>>>
>>> Add Siddharth and myself as reviewers.
>>>
>>> Signed-off-by: Ravi Gunasekaran <r-gunasekaran@ti.com>
>>> ---
>>> MAINTAINERS | 3 ++-
>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index 7b151710e8c5..bd52c33bca02 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -21775,7 +21775,8 @@ F: Documentation/devicetree/bindings/counter/ti-eqep.yaml
>>> F: drivers/counter/ti-eqep.c
>>>
>>> TI ETHERNET SWITCH DRIVER (CPSW)
>>> -R: Grygorii Strashko <grygorii.strashko@ti.com>
>>> +R: Siddharth Vadapalli <s-vadapalli@ti.com>
>>> +R: Ravi Gunasekaran <r-gunasekaran@ti.com>
>>
>> Could you please add me as Reviewer as well. Thanks!
>
> Thanks for volunteering to be a reviewer.
>
> I posted a v2 adding you as a reviewer.
> https://lore.kernel.org/all/20231110092749.3618-1-r-gunasekaran@ti.com/
>
>>
>>> L: linux-omap@vger.kernel.org
>>> L: netdev@vger.kernel.org
>>> S: Maintained
>>
>>> F: drivers/net/ethernet/ti/cpsw*
>>> F: drivers/net/ethernet/ti/davinci*
>>
>> What about am65-cpsw*?
>>
>> And drivers/net/ethernet/ti/icssg/*
>
> I would prefer a separate entry for ICSSG. Will let Danish comment on this.
>
Sure, I will share a separate patch for this. Roger, do you want me to
add you as a reviewer in for ICSSG driver as well?
>>
>> I also see
>>
>> OMAP GPIO DRIVER
>> M: Grygorii Strashko <grygorii.strashko@ti.com>
>>
>> Maybe a separate patch to remove the invalid email-id?
>>
> Yes, that's the plan. One of us from TI would be posting shortly.
>
>
--
Thanks and Regards,
Md Danish Anwar
^ permalink raw reply
* [PATCH net] tipc: Fix kernel-infoleak due to uninitialized TLV value
From: Shigeru Yoshida @ 2023-11-10 16:39 UTC (permalink / raw)
To: jmaloy, ying.xue, davem, edumazet, kuba, pabeni
Cc: netdev, tipc-discussion, linux-kernel, Shigeru Yoshida
KMSAN reported the following kernel-infoleak issue:
=====================================================
BUG: KMSAN: kernel-infoleak in instrument_copy_to_user include/linux/instrumented.h:114 [inline]
BUG: KMSAN: kernel-infoleak in copy_to_user_iter lib/iov_iter.c:24 [inline]
BUG: KMSAN: kernel-infoleak in iterate_ubuf include/linux/iov_iter.h:29 [inline]
BUG: KMSAN: kernel-infoleak in iterate_and_advance2 include/linux/iov_iter.h:245 [inline]
BUG: KMSAN: kernel-infoleak in iterate_and_advance include/linux/iov_iter.h:271 [inline]
BUG: KMSAN: kernel-infoleak in _copy_to_iter+0x4ec/0x2bc0 lib/iov_iter.c:186
instrument_copy_to_user include/linux/instrumented.h:114 [inline]
copy_to_user_iter lib/iov_iter.c:24 [inline]
iterate_ubuf include/linux/iov_iter.h:29 [inline]
iterate_and_advance2 include/linux/iov_iter.h:245 [inline]
iterate_and_advance include/linux/iov_iter.h:271 [inline]
_copy_to_iter+0x4ec/0x2bc0 lib/iov_iter.c:186
copy_to_iter include/linux/uio.h:197 [inline]
simple_copy_to_iter net/core/datagram.c:532 [inline]
__skb_datagram_iter.5+0x148/0xe30 net/core/datagram.c:420
skb_copy_datagram_iter+0x52/0x210 net/core/datagram.c:546
skb_copy_datagram_msg include/linux/skbuff.h:3960 [inline]
netlink_recvmsg+0x43d/0x1630 net/netlink/af_netlink.c:1967
sock_recvmsg_nosec net/socket.c:1044 [inline]
sock_recvmsg net/socket.c:1066 [inline]
__sys_recvfrom+0x476/0x860 net/socket.c:2246
__do_sys_recvfrom net/socket.c:2264 [inline]
__se_sys_recvfrom net/socket.c:2260 [inline]
__x64_sys_recvfrom+0x130/0x200 net/socket.c:2260
do_syscall_x64 arch/x86/entry/common.c:51 [inline]
do_syscall_64+0x44/0x110 arch/x86/entry/common.c:82
entry_SYSCALL_64_after_hwframe+0x63/0x6b
Uninit was created at:
slab_post_alloc_hook+0x103/0x9e0 mm/slab.h:768
slab_alloc_node mm/slub.c:3478 [inline]
kmem_cache_alloc_node+0x5f7/0xb50 mm/slub.c:3523
kmalloc_reserve+0x13c/0x4a0 net/core/skbuff.c:560
__alloc_skb+0x2fd/0x770 net/core/skbuff.c:651
alloc_skb include/linux/skbuff.h:1286 [inline]
tipc_tlv_alloc net/tipc/netlink_compat.c:156 [inline]
tipc_get_err_tlv+0x90/0x5d0 net/tipc/netlink_compat.c:170
tipc_nl_compat_recv+0x1042/0x15d0 net/tipc/netlink_compat.c:1324
genl_family_rcv_msg_doit net/netlink/genetlink.c:972 [inline]
genl_family_rcv_msg net/netlink/genetlink.c:1052 [inline]
genl_rcv_msg+0x1220/0x12c0 net/netlink/genetlink.c:1067
netlink_rcv_skb+0x4a4/0x6a0 net/netlink/af_netlink.c:2545
genl_rcv+0x41/0x60 net/netlink/genetlink.c:1076
netlink_unicast_kernel net/netlink/af_netlink.c:1342 [inline]
netlink_unicast+0xf4b/0x1230 net/netlink/af_netlink.c:1368
netlink_sendmsg+0x1242/0x1420 net/netlink/af_netlink.c:1910
sock_sendmsg_nosec net/socket.c:730 [inline]
__sock_sendmsg net/socket.c:745 [inline]
____sys_sendmsg+0x997/0xd60 net/socket.c:2588
___sys_sendmsg+0x271/0x3b0 net/socket.c:2642
__sys_sendmsg net/socket.c:2671 [inline]
__do_sys_sendmsg net/socket.c:2680 [inline]
__se_sys_sendmsg net/socket.c:2678 [inline]
__x64_sys_sendmsg+0x2fa/0x4a0 net/socket.c:2678
do_syscall_x64 arch/x86/entry/common.c:51 [inline]
do_syscall_64+0x44/0x110 arch/x86/entry/common.c:82
entry_SYSCALL_64_after_hwframe+0x63/0x6b
Bytes 34-35 of 36 are uninitialized
Memory access of size 36 starts at ffff88802d464a00
Data copied to user address 00007ff55033c0a0
CPU: 0 PID: 30322 Comm: syz-executor.0 Not tainted 6.6.0-14500-g1c41041124bd #10
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-1.fc38 04/01/2014
=====================================================
tipc_add_tlv() puts TLV descriptor and value onto `skb`. This size is
calculated with TLV_SPACE() macro. It adds the size of struct tlv_desc and
the length of TLV value passed as an argument, and aligns the result to a
multiple of TLV_ALIGNTO, i.e., a multiple of 4 bytes.
If the size of struct tlv_desc plus the length of TLV value is not aligned,
the current implementation leaves the remaining bytes uninitialized. This
is the cause of the above kernel-infoleak issue.
This patch resolves this issue by clearing data up to an aligned size.
Fixes: d0796d1ef63d ("tipc: convert legacy nl bearer dump to nl compat")
Signed-off-by: Shigeru Yoshida <syoshida@redhat.com>
---
net/tipc/netlink_compat.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/tipc/netlink_compat.c b/net/tipc/netlink_compat.c
index 5bc076f2fa74..c763008a8adb 100644
--- a/net/tipc/netlink_compat.c
+++ b/net/tipc/netlink_compat.c
@@ -102,6 +102,7 @@ static int tipc_add_tlv(struct sk_buff *skb, u16 type, void *data, u16 len)
return -EMSGSIZE;
skb_put(skb, TLV_SPACE(len));
+ memset(tlv, 0, TLV_SPACE(len));
tlv->tlv_type = htons(type);
tlv->tlv_len = htons(TLV_LENGTH(len));
if (len && data)
--
2.41.0
^ permalink raw reply related
* Re: [PATCH net] ptp: annotate data-race around q->head and q->tail
From: Richard Cochran @ 2023-11-10 17:09 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, netdev,
eric.dumazet
In-Reply-To: <CANn89iL5NC4-auwBRAitOiGMEk1Ewo9LOu2TitYHnU3ekzAaeA@mail.gmail.com>
On Fri, Nov 10, 2023 at 10:42:01AM +0100, Eric Dumazet wrote:
> I do not see how races are solved... Shouldn't
> pccontext->private_clkdata be protected by RCU ?
Yeah, the test is useless, because the memory is allocated in open()
and later freed in release(). During ioctl() the pointer must be
valid.
However, there was a bogus call to ptp_release() in the read() method,
but that has now been removed, and so the test is now bogus.
Thanks,
Richard
^ permalink raw reply
* Re: [PATCH net v2] ncsi: Revert NCSI link loss/gain commit
From: Simon Horman @ 2023-11-10 17:31 UTC (permalink / raw)
To: Johnathan Mantey; +Cc: netdev, sam, edumazet, kuba, pabeni, linux-kernel
In-Reply-To: <20231109205137.819392-1-johnathanx.mantey@intel.com>
On Thu, Nov 09, 2023 at 12:51:37PM -0800, Johnathan Mantey wrote:
> The NCSI commit
> ncsi: Propagate carrier gain/loss events to the NCSI controller
> introduced unwanted behavior.
>
> The intent for the commit was to be able to detect carrier loss/gain
> for just the NIC connected to the BMC. The unwanted effect is a
> carrier loss for auxiliary paths also causes the BMC to lose
> carrier. The BMC never regains carrier despite the secondary NIC
> regaining a link.
>
> This change, when merged, needs to be backported to stable kernels.
> 5.4-stable, 5.10-stable, 5.15-stable, 6.1-stable, 6.5-stable
>
> Fixes: 3780bb29311e ncsi: Propagate carrier gain/loss events to the
> CC: stable@vger.kernel.org
> Signed-off-by: Johnathan Mantey <johnathanx.mantey@intel.com>
Hi Johnathan,
thanks for your patch.
Some minor feedback from my side.
1. The correct format for the tag above is:
Fixes: 3780bb29311e ("ncsi: Propagate carrier gain/loss events to the NCSI controller")
2. I think it is usual to format the subject and commit messages for
revert commits a bit like this:
Subject: [PATCH net vX] Revert "ncsi: Propagate carrier gain/loss events to the NCSI controller"
This reverts commit 3780bb29311eccb7a1c9641032a112eed237f7e3.
The cited commit introduced unwanted behavior.
The intent for the commit was to be able to detect carrier loss/gain
for just the NIC connected to the BMC. The unwanted effect is a
carrier loss for auxiliary paths also causes the BMC to lose
carrier. The BMC never regains carrier despite the secondary NIC
regaining a link.
...
^ permalink raw reply
* [PATCH] net: memory leak in nr_rx_frame
From: Bragatheswaran Manickavel @ 2023-11-10 17:36 UTC (permalink / raw)
To: ralf, davem, edumazet, kuba, pabeni
Cc: Bragatheswaran Manickavel, linux-hams, netdev, linux-kernel,
syzbot+0145ea560de205bc09f0
The condition (make = nr_make_new(sk)) == NULL suggests
that nr_make_new allocates memory and returns a pointer.
If this allocation fails (returns NULL), it indicates a
potential memory leak.
Added sock_put() for make which can potentially solve
this issue
Reported-by: syzbot+0145ea560de205bc09f0@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=0145ea560de205bc09f0
Signed-off-by: Bragatheswaran Manickavel <bragathemanick0908@gmail.com>
---
net/netrom/af_netrom.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/netrom/af_netrom.c b/net/netrom/af_netrom.c
index 0eed00184adf..7d7cda4ae300 100644
--- a/net/netrom/af_netrom.c
+++ b/net/netrom/af_netrom.c
@@ -970,6 +970,8 @@ int nr_rx_frame(struct sk_buff *skb, struct net_device *dev)
nr_transmit_refusal(skb, 0);
if (sk)
sock_put(sk);
+ if (make)
+ sock_put(make);
return 0;
}
--
2.34.1
^ permalink raw reply related
* Re: [PATCH] netlink: introduce netlink poll to resolve fast return issue
From: Jakub Kicinski @ 2023-11-10 19:00 UTC (permalink / raw)
To: Jong eon Park
Cc: 'Paolo Abeni', 'David S. Miller',
'Eric Dumazet', netdev, linux-kernel,
'Dong ha Kang'
In-Reply-To: <000001da13e5$d9b99e30$8d2cda90$@samsung.com>
On Fri, 10 Nov 2023 23:54:48 +0900 Jong eon Park wrote:
> Interestingly, in this issue, even though netlink overrun frequently
> happened and caused POLLERRs, the user was managing it well through
> POLLIN and 'recv' function without a specific POLLERR handler.
> However, in the current situation, rcv queue is already empty and
> NETLINK_S_CONGESTED flag prevents any more incoming packets. This makes
> it impossible for the user to call 'recv'.
>
> This "congested" situation is a bit ambiguous. The queue is empty, yet
> 'congested' remains. This means kernel can no longer deliver uevents
> despite the empty queue, and it lead to the persistent 'congested' status.
>
> The reason for the difference in netlink lies in the NETLINK_S_CONGESTED
> flag. If it were UDP, upon seeing the empty queue, it might have kept
> pushing the received packets into the queue (making possible to call
> 'recv').
I see, please add a comment saying that NETLINK_S_CONGESTED prevents
new skbs from being queued before the new test in netlink_poll().
Please repost next week (i.e. after the merge window) with subject
tagged [PATCH net-next v2].
^ permalink raw reply
* Re: [PATCH v2] Fixes the null pointer deferences in nsim_bpf
From: Jakub Kicinski @ 2023-11-10 19:12 UTC (permalink / raw)
To: Dipendra Khadka
Cc: davem, edumazet, pabeni, netdev, linux-kernel,
linux-kernel-mentees, syzbot+44c2416196b7c607f226,
Stanislav Fomichev
In-Reply-To: <20231110111823.2775-1-kdipendra88@gmail.com>
On Fri, 10 Nov 2023 11:18:23 +0000 Dipendra Khadka wrote:
> Syzkaller found a null pointer dereference in nsim_bpf
> originating from the lack of a null check for state.
>
> This patch fixes the issue by adding a check for state
> in two functions nsim_prog_set_loaded() and nsim_setup_prog_hw_checks()
>
> Reported-by: syzbot+44c2416196b7c607f226@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com./bug?extid=44c2416196b7c607f226
> Fixes: 31d3ad832948 ("netdevsim: add bpf offload support")
Don't think so. It's probably due to Stan's extensions / reuse of
the offload infra.
Please put more effort into figuring out when and why this started
happening. Describe your findings in the commit message.
Current patch looks too much like a bandaid.
Before you repost read:
https://www.kernel.org/doc/html/next/process/maintainer-netdev.html
--
pw-bot: cr
pv-bot: syz
pv-bot: 24h
^ permalink raw reply
* Re: [PATCH] tls: fix missing memory barrier in tls_init
From: kernel test robot @ 2023-11-10 19:15 UTC (permalink / raw)
To: Dae R. Jeong, borisp, john.fastabend, kuba, davem, edumazet,
pabeni, netdev, linux-kernel
Cc: oe-kbuild-all, ywchoi
In-Reply-To: <ZU4Mk_RfzvRpwkmX@dragonet>
Hi Dae,
kernel test robot noticed the following build errors:
[auto build test ERROR on linus/master]
[also build test ERROR on v6.6 next-20231110]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Dae-R-Jeong/tls-fix-missing-memory-barrier-in-tls_init/20231110-190047
base: linus/master
patch link: https://lore.kernel.org/r/ZU4Mk_RfzvRpwkmX%40dragonet
patch subject: [PATCH] tls: fix missing memory barrier in tls_init
config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20231111/202311110243.1Rkt9wSg-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231111/202311110243.1Rkt9wSg-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202311110243.1Rkt9wSg-lkp@intel.com/
All errors (new ones prefixed by >>):
alpha-linux-ld: net/tls/tls_toe.o: in function `tls_toe_bypass':
>> (.text+0x280): undefined reference to `tls_ctx_create'
>> alpha-linux-ld: (.text+0x288): undefined reference to `tls_ctx_create'
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH v2 net 4/7] net/sched: taprio: get corrected value of cycle_time and interval
From: kernel test robot @ 2023-11-10 19:15 UTC (permalink / raw)
To: Faizal Rahim, Vladimir Oltean, Vinicius Costa Gomes,
Jamal Hadi Salim, Cong Wang, Jiri Pirko, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: llvm, oe-kbuild-all, netdev, linux-kernel
In-Reply-To: <20231107112023.676016-5-faizal.abdul.rahim@linux.intel.com>
Hi Faizal,
kernel test robot noticed the following build warnings:
[auto build test WARNING on net/main]
url: https://github.com/intel-lab-lkp/linux/commits/Faizal-Rahim/net-sched-taprio-fix-too-early-schedules-switching/20231107-192843
base: net/main
patch link: https://lore.kernel.org/r/20231107112023.676016-5-faizal.abdul.rahim%40linux.intel.com
patch subject: [PATCH v2 net 4/7] net/sched: taprio: get corrected value of cycle_time and interval
config: arm64-allmodconfig (https://download.01.org/0day-ci/archive/20231111/202311110208.GT4trtEk-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231111/202311110208.GT4trtEk-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202311110208.GT4trtEk-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> net/sched/sch_taprio.c:227:5: warning: no previous prototype for function 'get_interval' [-Wmissing-prototypes]
227 | u32 get_interval(const struct sched_entry *entry,
| ^
net/sched/sch_taprio.c:227:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
227 | u32 get_interval(const struct sched_entry *entry,
| ^
| static
>> net/sched/sch_taprio.c:236:5: warning: no previous prototype for function 'get_cycle_time' [-Wmissing-prototypes]
236 | s64 get_cycle_time(const struct sched_gate_list *oper)
| ^
net/sched/sch_taprio.c:236:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
236 | s64 get_cycle_time(const struct sched_gate_list *oper)
| ^
| static
2 warnings generated.
vim +/get_interval +227 net/sched/sch_taprio.c
226
> 227 u32 get_interval(const struct sched_entry *entry,
228 const struct sched_gate_list *oper)
229 {
230 if (entry->correction_active)
231 return entry->interval + oper->cycle_time_correction;
232 else
233 return entry->interval;
234 }
235
> 236 s64 get_cycle_time(const struct sched_gate_list *oper)
237 {
238 if (cycle_corr_active(oper->cycle_time_correction))
239 return oper->cycle_time + oper->cycle_time_correction;
240 else
241 return oper->cycle_time;
242 }
243
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH v2] Fixes the null pointer deferences in nsim_bpf
From: Stanislav Fomichev @ 2023-11-10 19:20 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Dipendra Khadka, davem, edumazet, pabeni, netdev, linux-kernel,
linux-kernel-mentees, syzbot+44c2416196b7c607f226,
Stanislav Fomichev
In-Reply-To: <20231110111223.692adbd7@kernel.org>
On 11/10, Jakub Kicinski wrote:
> On Fri, 10 Nov 2023 11:18:23 +0000 Dipendra Khadka wrote:
> > Syzkaller found a null pointer dereference in nsim_bpf
> > originating from the lack of a null check for state.
> >
> > This patch fixes the issue by adding a check for state
> > in two functions nsim_prog_set_loaded() and nsim_setup_prog_hw_checks()
> >
> > Reported-by: syzbot+44c2416196b7c607f226@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com./bug?extid=44c2416196b7c607f226
> > Fixes: 31d3ad832948 ("netdevsim: add bpf offload support")
>
> Don't think so. It's probably due to Stan's extensions / reuse of
> the offload infra.
>
> Please put more effort into figuring out when and why this started
> happening. Describe your findings in the commit message.
>
> Current patch looks too much like a bandaid.
>
> Before you repost read:
> https://www.kernel.org/doc/html/next/process/maintainer-netdev.html
I agree, I have a similar suspicion for the same report on the bpf list [0].
0: https://lore.kernel.org/bpf/ZU13dQb2z66CJlYi@google.com/
^ permalink raw reply
* Re: [RFC net-next] net: don't dump stack on queue timeout
From: Jakub Kicinski @ 2023-11-10 19:21 UTC (permalink / raw)
To: Daniele Palmas
Cc: davem, netdev, edumazet, pabeni, syzbot+d55372214aff0faa1f1f, jhs,
xiyou.wangcong, jiri
In-Reply-To: <CAGRyCJFLytO-k1ekbQE5Z3LN7RVJciB_4Yh9PUVYA3EZeWMG5A@mail.gmail.com>
On Fri, 10 Nov 2023 09:01:29 +0100 Daniele Palmas wrote:
> The problem is that the MBIM standard does not define the
> CDC_NOTIFY_NETWORK_CONNECTION, so carrier loss detection is managed
> through the indications on the control channel.
>
> But the kernel is not aware of what's passing through the control
> channel, so it's the userspace tool that should detect carrier loss,
> disconnect the bearers and set the network interface down.
>
> For example, ModemManager is capable of doing that, but the problem is
> that usually the standard modem notifications on the control channel
> arrive later than the splat: increasing watchdog_timeo does not seem
> to me a good option, since the notification could arrive much later.
>
> One possible solution is to have some proprietary notifications on the
> control channel that detect RLF early and trigger the above described
> process before the warn happens: by coincidence, I wrote a custom
> ModemManager patch for this a few days ago
> https://gitlab.freedesktop.org/dnlplm/ModemManager/-/commit/89ba8ab65d4bfbd4cf1ff11ed58c08b112aca80f
I see, thanks for the extra info!
^ permalink raw reply
* Re: [PATCH] tls: fix missing memory barrier in tls_init
From: Jakub Kicinski @ 2023-11-10 19:44 UTC (permalink / raw)
To: Dae R. Jeong
Cc: borisp, john.fastabend, davem, edumazet, pabeni, netdev,
linux-kernel, ywchoi
In-Reply-To: <ZU4Mk_RfzvRpwkmX@dragonet>
On Fri, 10 Nov 2023 19:57:23 +0900 Dae R. Jeong wrote:
> + mutex_init(&ctx->tx_lock);
> + ctx->sk_proto = READ_ONCE(sk->sk_prot);
> + ctx->sk = sk;
> ctx->tx_conf = TLS_BASE;
> ctx->rx_conf = TLS_BASE;
TLS_BASE is 0, so there's no strong reason to move the rcu assign
after *x_conf init. It's already 0. You can replace the assignment
with WARN_ON(ctx->tx_conf != TLS_BASE) to make sure, and move that
into tls_ctx_create() instead of removing that function.
FWIW make sure you read this before posting v2:
https://www.kernel.org/doc/html/next/process/maintainer-netdev.html
^ permalink raw reply
* Re: [PATCH net] ptp: annotate data-race around q->head and q->tail
From: Jakub Kicinski @ 2023-11-10 19:52 UTC (permalink / raw)
To: Richard Cochran
Cc: Eric Dumazet, David S . Miller, Paolo Abeni, netdev, eric.dumazet
In-Reply-To: <ZU5j2V9aUae0FE1o@hoboy.vegasvil.org>
On Fri, 10 Nov 2023 09:09:45 -0800 Richard Cochran wrote:
> > I do not see how races are solved... Shouldn't
> > pccontext->private_clkdata be protected by RCU ?
>
> Yeah, the test is useless, because the memory is allocated in open()
> and later freed in release(). During ioctl() the pointer must be
> valid.
>
> However, there was a bogus call to ptp_release() in the read() method,
> but that has now been removed, and so the test is now bogus.
Meaning we should revert 8a4f030dbced ("ptp: Fixes a null pointer
dereference in ptp_ioctl") ?
^ permalink raw reply
* Re: [net-next RFC PATCH v6 3/4] net: phy: aquantia: add firmware load support
From: Simon Horman @ 2023-11-10 19:57 UTC (permalink / raw)
To: Christian Marangi
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Andrew Lunn,
Heiner Kallweit, Russell King, Robert Marko, Vladimir Oltean,
netdev, devicetree, linux-kernel
In-Reply-To: <20231109123253.3933-3-ansuelsmth@gmail.com>
On Thu, Nov 09, 2023 at 01:32:52PM +0100, Christian Marangi wrote:
> From: Robert Marko <robimarko@gmail.com>
>
> Aquantia PHY-s require firmware to be loaded before they start operating.
> It can be automatically loaded in case when there is a SPI-NOR connected
> to Aquantia PHY-s or can be loaded from the host via MDIO.
>
> This patch adds support for loading the firmware via MDIO as in most cases
> there is no SPI-NOR being used to save on cost.
> Firmware loading code itself is ported from mainline U-boot with cleanups.
>
> The firmware has mixed values both in big and little endian.
> PHY core itself is big-endian but it expects values to be in little-endian.
> The firmware is little-endian but CRC-16 value for it is stored at the end
> of firmware in big-endian.
>
> It seems the PHY does the conversion internally from firmware that is
> little-endian to the PHY that is big-endian on using the mailbox
> but mailbox returns a big-endian CRC-16 to verify the written data
> integrity.
>
> Co-developed-by: Christian Marangi <ansuelsmth@gmail.com>
> Signed-off-by: Robert Marko <robimarko@gmail.com>
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Hi Christian and Robert,
thanks for your patch-set.
I spotted some minor endien issues which I have highlighted below.
...
> +/* load data into the phy's memory */
> +static int aqr_fw_load_memory(struct phy_device *phydev, u32 addr,
> + const u8 *data, size_t len)
> +{
> + u16 crc = 0, up_crc;
> + size_t pos;
> +
> + /* PHY expect addr in LE */
> + addr = cpu_to_le32(addr);
The type of addr is host byte-order,
but here it is assigned a little-endian value.
Flagged by Sparse.
> +
> + phy_write_mmd(phydev, MDIO_MMD_VEND1,
> + VEND1_GLOBAL_MAILBOX_INTERFACE1,
> + VEND1_GLOBAL_MAILBOX_INTERFACE1_CRC_RESET);
> + phy_write_mmd(phydev, MDIO_MMD_VEND1,
> + VEND1_GLOBAL_MAILBOX_INTERFACE3,
> + VEND1_GLOBAL_MAILBOX_INTERFACE3_MSW_ADDR(addr));
VEND1_GLOBAL_MAILBOX_INTERFACE3_MSW_ADDR() performs a bit-shift on addr,
and applies a mask which is in host-byte order.
But, as highlighted above, addr is a little-endian value.
This does not seem right.
This is all hidden by a cast in VEND1_GLOBAL_MAILBOX_INTERFACE3_MSW_ADDR()
This seems dangerous to me.
> + phy_write_mmd(phydev, MDIO_MMD_VEND1,
> + VEND1_GLOBAL_MAILBOX_INTERFACE4,
> + VEND1_GLOBAL_MAILBOX_INTERFACE4_LSW_ADDR(addr));
There seem to be similar issues with the use of addr here.
> +
> + /* We assume and enforce the size to be word aligned.
> + * If a firmware that is not word aligned is found, please report upstream.
> + */
> + for (pos = 0; pos < len; pos += sizeof(u32)) {
> + u32 word = get_unaligned((const u32 *)(data + pos));
> +
> + phy_write_mmd(phydev, MDIO_MMD_VEND1, VEND1_GLOBAL_MAILBOX_INTERFACE5,
> + VEND1_GLOBAL_MAILBOX_INTERFACE5_MSW_DATA(word));
> + phy_write_mmd(phydev, MDIO_MMD_VEND1, VEND1_GLOBAL_MAILBOX_INTERFACE6,
> + VEND1_GLOBAL_MAILBOX_INTERFACE6_LSW_DATA(word));
> +
> + phy_write_mmd(phydev, MDIO_MMD_VEND1, VEND1_GLOBAL_MAILBOX_INTERFACE1,
> + VEND1_GLOBAL_MAILBOX_INTERFACE1_EXECUTE |
> + VEND1_GLOBAL_MAILBOX_INTERFACE1_WRITE);
> +
> + /* calculate CRC as we load data to the mailbox.
> + * We convert word to big-endiang as PHY is BE and mailbox will
> + * return a BE CRC.
> + */
> + word = cpu_to_be32(word);
Similarly here, Sparse flags that a little-endian value is assigned to a
host byte-order variable.
> + crc = crc_ccitt_false(crc, (u8 *)&word, sizeof(word));
> + }
...
pw-bot: changes-requested
^ permalink raw reply
* Re: [PATCH net-next v4] hv_netvsc: Mark VF as slave before exposing it to user-mode
From: Jakub Kicinski @ 2023-11-10 20:05 UTC (permalink / raw)
To: Long Li
Cc: longli@linuxonhyperv.com, KY Srinivasan, Haiyang Zhang, Wei Liu,
Dexuan Cui, David S. Miller, Eric Dumazet, Paolo Abeni,
linux-hyperv@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <PH7PR21MB3263EBCF9600EEBD6D962B6ECEAEA@PH7PR21MB3263.namprd21.prod.outlook.com>
On Fri, 10 Nov 2023 00:43:55 +0000 Long Li wrote:
> The code above needs to work with and without netvsc (the possible
> master device) present.
I don't think that's a reasonable requirement for the kernel code.
The auto-bonding already puts the kernel into business of guessing
policy, which frankly we shouldn't be in.
Having the kernel guess even harder that there will be a master,
but it's not there yet, is not reasonable.
^ permalink raw reply
* Re: [RFC PATCH 1/8] dt-bindings: phy: mediatek,xfi-pextp: add new bindings
From: Rob Herring @ 2023-11-10 20:08 UTC (permalink / raw)
To: Daniel Golle
Cc: Andrew Lunn, Lorenzo Bianconi, Alexander Couzens, linux-kernel,
linux-mediatek, David S. Miller, Krzysztof Kozlowski,
Jakub Kicinski, Felix Fietkau, Russell King, Paolo Abeni,
Mark Lee, netdev, linux-arm-kernel, Conor Dooley, Vinod Koul,
Rob Herring, Kishon Vijay Abraham I, Heiner Kallweit, devicetree,
Sean Wang, Philipp Zabel, Chunfeng Yun, linux-phy,
Matthias Brugger, John Crispin, AngeloGioacchino Del Regno,
Eric Dumazet
In-Reply-To: <924c2c6316e6d51a17423eded3a2c5c5bbf349d2.1699565880.git.daniel@makrotopia.org>
On Thu, 09 Nov 2023 21:50:55 +0000, Daniel Golle wrote:
> Add bindings for the MediaTek PEXTP Ethernet SerDes PHY found in the
> MediaTek MT7988 SoC which can operate at various interfaces modes:
>
> * USXGMII
> * 10GBase-R
> * 5GBase-R
> * 2500Base-X
> * 1000Base-X
> * Cisco SGMII (MAC side)
>
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> ---
> .../bindings/phy/mediatek,xfi-pextp.yaml | 71 +++++++++++++++++++
> 1 file changed, 71 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/phy/mediatek,xfi-pextp.yaml
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/phy/mediatek,xfi-pextp.example.dts:18:18: fatal error: dt-bindings/clock/mediatek,mt7988-clk.h: No such file or directory
18 | #include <dt-bindings/clock/mediatek,mt7988-clk.h>
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[2]: *** [scripts/Makefile.lib:419: Documentation/devicetree/bindings/phy/mediatek,xfi-pextp.example.dtb] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1427: dt_binding_check] Error 2
make: *** [Makefile:234: __sub-make] Error 2
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/924c2c6316e6d51a17423eded3a2c5c5bbf349d2.1699565880.git.daniel@makrotopia.org
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
^ permalink raw reply
* [PATCH net 0/2] pds_core: fix irq index bug and compiler warnings
From: Shannon Nelson @ 2023-11-10 20:07 UTC (permalink / raw)
To: netdev, brett.creeley, davem, edumazet, kuba, pabeni
Cc: drivers, joao.m.martins, Shannon Nelson
The first patch fixes a bug in our interrupt masking where we used the
wrong index. The second patch addresses a couple of kernel test robot
string truncation warnings.
Shannon Nelson (2):
pds_core: use correct index to mask irq
pds_core: fix up some format-truncation complaints
drivers/net/ethernet/amd/pds_core/adminq.c | 2 +-
drivers/net/ethernet/amd/pds_core/core.h | 2 +-
drivers/net/ethernet/amd/pds_core/dev.c | 8 ++++++--
drivers/net/ethernet/amd/pds_core/devlink.c | 2 +-
4 files changed, 9 insertions(+), 5 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH net 1/2] pds_core: use correct index to mask irq
From: Shannon Nelson @ 2023-11-10 20:07 UTC (permalink / raw)
To: netdev, brett.creeley, davem, edumazet, kuba, pabeni
Cc: drivers, joao.m.martins, Shannon Nelson
In-Reply-To: <20231110200759.56770-1-shannon.nelson@amd.com>
Use the qcq's interrupt index, not the irq number, to mask
the interrupt.
Reported-by: Joao Martins <joao.m.martins@oracle.com>
Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
---
drivers/net/ethernet/amd/pds_core/adminq.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/amd/pds_core/adminq.c b/drivers/net/ethernet/amd/pds_core/adminq.c
index 045fe133f6ee..5beadabc2136 100644
--- a/drivers/net/ethernet/amd/pds_core/adminq.c
+++ b/drivers/net/ethernet/amd/pds_core/adminq.c
@@ -146,7 +146,7 @@ irqreturn_t pdsc_adminq_isr(int irq, void *data)
}
queue_work(pdsc->wq, &qcq->work);
- pds_core_intr_mask(&pdsc->intr_ctrl[irq], PDS_CORE_INTR_MASK_CLEAR);
+ pds_core_intr_mask(&pdsc->intr_ctrl[qcq->intx], PDS_CORE_INTR_MASK_CLEAR);
return IRQ_HANDLED;
}
--
2.17.1
^ permalink raw reply related
* [PATCH net 2/2] pds_core: fix up some format-truncation complaints
From: Shannon Nelson @ 2023-11-10 20:07 UTC (permalink / raw)
To: netdev, brett.creeley, davem, edumazet, kuba, pabeni
Cc: drivers, joao.m.martins, Shannon Nelson
In-Reply-To: <20231110200759.56770-1-shannon.nelson@amd.com>
Our friendly kernel test robot pointed out a couple of potential
string truncation issues. None of which were we worried about,
but can be relatively easily fixed to quiet the complaints.
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202310211736.66syyDpp-lkp@intel.com/
Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
---
drivers/net/ethernet/amd/pds_core/core.h | 2 +-
drivers/net/ethernet/amd/pds_core/dev.c | 8 ++++++--
drivers/net/ethernet/amd/pds_core/devlink.c | 2 +-
3 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/amd/pds_core/core.h b/drivers/net/ethernet/amd/pds_core/core.h
index f3a7deda9972..e35d3e7006bf 100644
--- a/drivers/net/ethernet/amd/pds_core/core.h
+++ b/drivers/net/ethernet/amd/pds_core/core.h
@@ -15,7 +15,7 @@
#define PDSC_DRV_DESCRIPTION "AMD/Pensando Core Driver"
#define PDSC_WATCHDOG_SECS 5
-#define PDSC_QUEUE_NAME_MAX_SZ 32
+#define PDSC_QUEUE_NAME_MAX_SZ 16
#define PDSC_ADMINQ_MIN_LENGTH 16 /* must be a power of two */
#define PDSC_NOTIFYQ_LENGTH 64 /* must be a power of two */
#define PDSC_TEARDOWN_RECOVERY false
diff --git a/drivers/net/ethernet/amd/pds_core/dev.c b/drivers/net/ethernet/amd/pds_core/dev.c
index 7c1b965d61a9..31940b857e0e 100644
--- a/drivers/net/ethernet/amd/pds_core/dev.c
+++ b/drivers/net/ethernet/amd/pds_core/dev.c
@@ -261,10 +261,14 @@ static int pdsc_identify(struct pdsc *pdsc)
struct pds_core_drv_identity drv = {};
size_t sz;
int err;
+ int n;
drv.drv_type = cpu_to_le32(PDS_DRIVER_LINUX);
- snprintf(drv.driver_ver_str, sizeof(drv.driver_ver_str),
- "%s %s", PDS_CORE_DRV_NAME, utsname()->release);
+ /* Catching the return quiets a Wformat-truncation complaint */
+ n = snprintf(drv.driver_ver_str, sizeof(drv.driver_ver_str),
+ "%s %s", PDS_CORE_DRV_NAME, utsname()->release);
+ if (n > sizeof(drv.driver_ver_str))
+ dev_dbg(pdsc->dev, "release name truncated, don't care\n");
/* Next let's get some info about the device
* We use the devcmd_lock at this level in order to
diff --git a/drivers/net/ethernet/amd/pds_core/devlink.c b/drivers/net/ethernet/amd/pds_core/devlink.c
index 57f88c8b37de..e9948ea5bbcd 100644
--- a/drivers/net/ethernet/amd/pds_core/devlink.c
+++ b/drivers/net/ethernet/amd/pds_core/devlink.c
@@ -104,7 +104,7 @@ int pdsc_dl_info_get(struct devlink *dl, struct devlink_info_req *req,
struct pds_core_fw_list_info fw_list;
struct pdsc *pdsc = devlink_priv(dl);
union pds_core_dev_comp comp;
- char buf[16];
+ char buf[32];
int listlen;
int err;
int i;
--
2.17.1
^ permalink raw reply related
* Re: [net-next RFC PATCH v6 4/4] dt-bindings: Document Marvell Aquantia PHY
From: Rob Herring @ 2023-11-10 20:15 UTC (permalink / raw)
To: Christian Marangi
Cc: Andrew Lunn, Conor Dooley, Rob Herring, Krzysztof Kozlowski,
netdev, David S. Miller, Heiner Kallweit, devicetree,
linux-kernel, Conor Dooley, Jakub Kicinski, Russell King,
Robert Marko, Eric Dumazet, Paolo Abeni, Vladimir Oltean
In-Reply-To: <20231109123253.3933-4-ansuelsmth@gmail.com>
On Thu, 09 Nov 2023 13:32:53 +0100, Christian Marangi wrote:
> Document bindings for Marvell Aquantia PHY.
>
> The Marvell Aquantia PHY require a firmware to work correctly and there
> at least 3 way to load this firmware.
>
> Describe all the different way and document the binding "firmware-name"
> to load the PHY firmware from userspace.
>
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> ---
> Changes v6:
> - Add Reviewed-by tag
> - Drop comments in dts examples
> - Improve commit title
> - Fix wrong reg in example
> Changes v5:
> - Drop extra entry not related to HW description
> Changes v3:
> - Make DT description more OS agnostic
> - Use custom select to fix dtbs checks
> Changes v2:
> - Add DT patch
>
> .../bindings/net/marvell,aquantia.yaml | 116 ++++++++++++++++++
> 1 file changed, 116 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/marvell,aquantia.yaml
>
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH iproute2] Revert "Makefile: ensure CONF_USR_DIR honours the libdir config"
From: Andrea Claudi @ 2023-11-10 20:31 UTC (permalink / raw)
To: Petr Machata; +Cc: luca.boccassi, netdev, stephen
In-Reply-To: <87bkc1yaqa.fsf@nvidia.com>
On Fri, Nov 10, 2023 at 02:54:16PM +0100, Petr Machata wrote:
>
> Petr Machata <petrm@nvidia.com> writes:
>
> > luca.boccassi@gmail.com writes:
> >
> >> From: Luca Boccassi <bluca@debian.org>
> >>
> >> LIBDIR in Debian and derivatives is not /usr/lib/, it's
> >> /usr/lib/<architecture triplet>/, which is different, and it's the
> >> wrong location where to install architecture-independent default
> >> configuration files, which should always go to /usr/lib/ instead.
> >> Installing these files to the per-architecture directory is not
> >> the right thing, hence revert the change.
> >
> > So I looked into the Fedora package. Up until recently, the files were
> > in /etc, but it seems there was a deliberate change in the spec file
> > this September that moved them to /usr/lib or /usr/lib64.
> >
> > Luca -- since you both sent the patch under reversion, and are Fedora
>
> Ugh, I mean Andrea, not Luca. Sorry!
>
> > maintainer, could you please elaborate on what the logic was behind it?
> > It does look odd to me to put config files into an arch-dependent
> > directory, but I've been out of packaging for close to a decade at this
> > point.
Hi Petr,
the change in Fedora iproute package is in response to 0a0a8f12fa1b
("Read configuration files from /etc and /usr"): it moves config files
from /etc to /usr to make room for customization using /etc/iproute2, as
described over there.
What I tried to achieve with my patch is to have a single location in
/usr for iproute files; but I agree with both you and Luca that storing
config files in an arch-dependent directory doesn't look right.
However, even using /usr/lib doesn't seems quite right to me. According
to the FHS [1]:
"/usr/lib includes object files and libraries. On some systems, it may
also include internal binaries that are not intended to be executed
directly by users or shell scripts."
A better location is probably /usr/share [2]:
"The /usr/share hierarchy is for all read-only architecture independent
data files.
This hierarchy is intended to be shareable among all architecture
platforms of a given OS; thus, for example, a site with i386, Alpha, and
PPC platforms might maintain a single /usr/share directory that is
centrally-mounted."
And this is exactly our case: read-only, shareable, config files that
can be overridden using /etc/iproute2.
Luca, does something along the lines below work for you? If so, I can
test and send a patch fixing my own stuff.
diff --git a/Makefile b/Makefile
index 5c559c8d..ec57bd4c 100644
--- a/Makefile
+++ b/Makefile
@@ -16,11 +16,11 @@ endif
PREFIX?=/usr
SBINDIR?=/sbin
+DATADIR?=$(PREFIX)/share
CONF_ETC_DIR?=/etc/iproute2
-CONF_USR_DIR?=$(LIBDIR)/iproute2
+CONF_USR_DIR?=$(DATADIR)/iproute2
NETNS_RUN_DIR?=/var/run/netns
NETNS_ETC_DIR?=/etc/netns
-DATADIR?=$(PREFIX)/share
HDRDIR?=$(PREFIX)/include/iproute2
DOCDIR?=$(DATADIR)/doc/iproute2
MANDIR?=$(DATADIR)/man
[1] https://refspecs.linuxfoundation.org/FHS_3.0/fhs/ch04s06.html
[2] https://refspecs.linuxfoundation.org/FHS_3.0/fhs/ch04s11.html
^ permalink raw reply related
* [PATCH bpf-next v4 1/2] bpf: add skcipher API support to TC/XDP programs
From: Vadim Fedorenko @ 2023-11-10 20:34 UTC (permalink / raw)
To: Vadim Fedorenko, Jakub Kicinski, Martin KaFai Lau,
Andrii Nakryiko, Alexei Starovoitov, Mykola Lysenko
Cc: Vadim Fedorenko, bpf, netdev, linux-crypto
Add crypto API support to BPF to be able to decrypt or encrypt packets
in TC/XDP BPF programs. Only symmetric key ciphers are supported for
now. Special care should be taken for initialization part of crypto algo
because crypto_alloc_sync_skcipher() doesn't work with preemtion
disabled, it can be run only in sleepable BPF program. Also async crypto
is not supported because of the very same issue - TC/XDP BPF programs
are not sleepable.
Signed-off-by: Vadim Fedorenko <vadfed@meta.com>
---
v3 -> v4:
- reuse __bpf_dynptr_data and remove own implementation
- use const __str to provide algorithm name
- use kfunc macroses to avoid compilator warnings
v2 -> v3:
- fix kdoc issues
v1 -> v2:
- use kmalloc in sleepable func, suggested by Alexei
- use __bpf_dynptr_is_rdonly() to check destination, suggested by Jakub
- use __bpf_dynptr_data_ptr() for all dynptr accesses
---
include/linux/bpf.h | 1 +
kernel/bpf/Makefile | 3 +
kernel/bpf/crypto.c | 265 ++++++++++++++++++++++++++++++++++++++++++
kernel/bpf/helpers.c | 2 +-
kernel/bpf/verifier.c | 1 +
5 files changed, 271 insertions(+), 1 deletion(-)
create mode 100644 kernel/bpf/crypto.c
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 4001d11be151..77934ab7421d 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1224,6 +1224,7 @@ int bpf_dynptr_check_size(u32 size);
u32 __bpf_dynptr_size(const struct bpf_dynptr_kern *ptr);
const void *__bpf_dynptr_data(const struct bpf_dynptr_kern *ptr, u32 len);
void *__bpf_dynptr_data_rw(const struct bpf_dynptr_kern *ptr, u32 len);
+bool __bpf_dynptr_is_rdonly(const struct bpf_dynptr_kern *ptr);
#ifdef CONFIG_BPF_JIT
int bpf_trampoline_link_prog(struct bpf_tramp_link *link, struct bpf_trampoline *tr);
diff --git a/kernel/bpf/Makefile b/kernel/bpf/Makefile
index f526b7573e97..e14b5834c477 100644
--- a/kernel/bpf/Makefile
+++ b/kernel/bpf/Makefile
@@ -41,6 +41,9 @@ obj-$(CONFIG_BPF_SYSCALL) += bpf_struct_ops.o
obj-$(CONFIG_BPF_SYSCALL) += cpumask.o
obj-${CONFIG_BPF_LSM} += bpf_lsm.o
endif
+ifeq ($(CONFIG_CRYPTO_SKCIPHER),y)
+obj-$(CONFIG_BPF_SYSCALL) += crypto.o
+endif
obj-$(CONFIG_BPF_PRELOAD) += preload/
obj-$(CONFIG_BPF_SYSCALL) += relo_core.o
diff --git a/kernel/bpf/crypto.c b/kernel/bpf/crypto.c
new file mode 100644
index 000000000000..4fe1470ad4b8
--- /dev/null
+++ b/kernel/bpf/crypto.c
@@ -0,0 +1,265 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright (c) 2023 Meta, Inc */
+#include <linux/bpf.h>
+#include <linux/bpf_mem_alloc.h>
+#include <linux/btf.h>
+#include <linux/btf_ids.h>
+#include <linux/filter.h>
+#include <linux/scatterlist.h>
+#include <linux/skbuff.h>
+#include <crypto/skcipher.h>
+
+/**
+ * struct bpf_crypto_skcipher_ctx - refcounted BPF sync skcipher context structure
+ * @tfm: The pointer to crypto_sync_skcipher struct.
+ * @rcu: The RCU head used to free the crypto context with RCU safety.
+ * @usage: Object reference counter. When the refcount goes to 0, the
+ * memory is released back to the BPF allocator, which provides
+ * RCU safety.
+ */
+struct bpf_crypto_skcipher_ctx {
+ struct crypto_sync_skcipher *tfm;
+ struct rcu_head rcu;
+ refcount_t usage;
+};
+
+__bpf_kfunc_start_defs();
+
+/**
+ * bpf_crypto_skcipher_ctx_create() - Create a mutable BPF crypto context.
+ *
+ * Allocates a crypto context that can be used, acquired, and released by
+ * a BPF program. The crypto context returned by this function must either
+ * be embedded in a map as a kptr, or freed with bpf_crypto_skcipher_ctx_release().
+ *
+ * bpf_crypto_skcipher_ctx_create() allocates memory using the BPF memory
+ * allocator, and will not block. It may return NULL if no memory is available.
+ * @algo__str: pointer to string representation of algorithm.
+ * @pkey: bpf_dynptr which holds cipher key to do crypto.
+ * @err: integer to store error code when NULL is returned
+ */
+__bpf_kfunc struct bpf_crypto_skcipher_ctx *
+bpf_crypto_skcipher_ctx_create(const char *algo__str, const struct bpf_dynptr_kern *pkey,
+ int *err)
+{
+ struct bpf_crypto_skcipher_ctx *ctx;
+ const u8 *key;
+ u32 key_len;
+
+ if (!crypto_has_skcipher(algo__str, CRYPTO_ALG_TYPE_SKCIPHER, CRYPTO_ALG_TYPE_MASK)) {
+ *err = -EOPNOTSUPP;
+ return NULL;
+ }
+
+ key_len = __bpf_dynptr_size(pkey);
+ if (!key_len) {
+ *err = -EINVAL;
+ return NULL;
+ }
+ key = __bpf_dynptr_data(pkey, key_len);
+ if (!key) {
+ *err = -EINVAL;
+ return NULL;
+ }
+
+ ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
+ if (!ctx) {
+ *err = -ENOMEM;
+ return NULL;
+ }
+
+ ctx->tfm = crypto_alloc_sync_skcipher(algo__str, 0, 0);
+ if (IS_ERR(ctx->tfm)) {
+ *err = PTR_ERR(ctx->tfm);
+ ctx->tfm = NULL;
+ goto err;
+ }
+
+ *err = crypto_sync_skcipher_setkey(ctx->tfm, key, key_len);
+ if (*err)
+ goto err;
+
+ refcount_set(&ctx->usage, 1);
+
+ return ctx;
+err:
+ if (ctx->tfm)
+ crypto_free_sync_skcipher(ctx->tfm);
+ kfree(ctx);
+
+ return NULL;
+}
+
+static void crypto_free_sync_skcipher_cb(struct rcu_head *head)
+{
+ struct bpf_crypto_skcipher_ctx *ctx;
+
+ ctx = container_of(head, struct bpf_crypto_skcipher_ctx, rcu);
+ crypto_free_sync_skcipher(ctx->tfm);
+ kfree(ctx);
+}
+
+/**
+ * bpf_crypto_skcipher_ctx_acquire() - Acquire a reference to a BPF crypto context.
+ * @ctx: The BPF crypto context being acquired. The ctx must be a trusted
+ * pointer.
+ *
+ * Acquires a reference to a BPF crypto context. The context returned by this function
+ * must either be embedded in a map as a kptr, or freed with
+ * bpf_crypto_skcipher_ctx_release().
+ */
+__bpf_kfunc struct bpf_crypto_skcipher_ctx *
+bpf_crypto_skcipher_ctx_acquire(struct bpf_crypto_skcipher_ctx *ctx)
+{
+ refcount_inc(&ctx->usage);
+ return ctx;
+}
+
+/**
+ * bpf_crypto_skcipher_ctx_release() - Release a previously acquired BPF crypto context.
+ * @ctx: The crypto context being released.
+ *
+ * Releases a previously acquired reference to a BPF cpumask. When the final
+ * reference of the BPF cpumask has been released, it is subsequently freed in
+ * an RCU callback in the BPF memory allocator.
+ */
+__bpf_kfunc void bpf_crypto_skcipher_ctx_release(struct bpf_crypto_skcipher_ctx *ctx)
+{
+ if (refcount_dec_and_test(&ctx->usage))
+ call_rcu(&ctx->rcu, crypto_free_sync_skcipher_cb);
+}
+
+static int bpf_crypto_skcipher_crypt(struct crypto_sync_skcipher *tfm,
+ const struct bpf_dynptr_kern *src,
+ struct bpf_dynptr_kern *dst,
+ const struct bpf_dynptr_kern *iv,
+ bool decrypt)
+{
+ SYNC_SKCIPHER_REQUEST_ON_STACK(req, tfm);
+ struct scatterlist sgin, sgout;
+ u32 src_len, dst_len, iv_len;
+ const u8 *psrc;
+ u8 *pdst, *piv;
+ int err;
+
+ if (crypto_sync_skcipher_get_flags(tfm) & CRYPTO_TFM_NEED_KEY)
+ return -EINVAL;
+
+ if (__bpf_dynptr_is_rdonly(dst))
+ return -EINVAL;
+
+ iv_len = __bpf_dynptr_size(iv);
+ src_len = __bpf_dynptr_size(src);
+ dst_len = __bpf_dynptr_size(dst);
+ if (!src_len || !dst_len)
+ return -EINVAL;
+
+ if (iv_len != crypto_sync_skcipher_ivsize(tfm))
+ return -EINVAL;
+
+ psrc = __bpf_dynptr_data(src, src_len);
+ if (!psrc)
+ return -EINVAL;
+ pdst = __bpf_dynptr_data_rw(dst, dst_len);
+ if (!pdst)
+ return -EINVAL;
+
+ piv = iv_len ? __bpf_dynptr_data_rw(iv, iv_len) : NULL;
+ if (iv_len && !piv)
+ return -EINVAL;
+
+ sg_init_one(&sgin, psrc, src_len);
+ sg_init_one(&sgout, pdst, dst_len);
+
+ skcipher_request_set_sync_tfm(req, tfm);
+ skcipher_request_set_callback(req, 0, NULL, NULL);
+ skcipher_request_set_crypt(req, &sgin, &sgout, src_len, piv);
+
+ err = decrypt ? crypto_skcipher_decrypt(req) : crypto_skcipher_encrypt(req);
+ skcipher_request_zero(req);
+
+ return err;
+}
+
+/**
+ * bpf_crypto_skcipher_decrypt() - Decrypt buffer using configured context and IV provided.
+ * @ctx: The crypto context being used. The ctx must be a trusted pointer.
+ * @src: bpf_dynptr to the encrypted data. Must be a trusted pointer.
+ * @dst: bpf_dynptr to the buffer where to store the result. Must be a trusted pointer.
+ * @iv: bpf_dynptr to IV data to be used by decryptor.
+ *
+ * Decrypts provided buffer using IV data and the crypto context. Crypto context must be configured.
+ */
+__bpf_kfunc int bpf_crypto_skcipher_decrypt(struct bpf_crypto_skcipher_ctx *ctx,
+ const struct bpf_dynptr_kern *src,
+ struct bpf_dynptr_kern *dst,
+ struct bpf_dynptr_kern *iv)
+{
+ return bpf_crypto_skcipher_crypt(ctx->tfm, src, dst, iv, true);
+}
+
+/**
+ * bpf_crypto_skcipher_encrypt() - Encrypt buffer using configured context and IV provided.
+ * @ctx: The crypto context being used. The ctx must be a trusted pointer.
+ * @src: bpf_dynptr to the plain data. Must be a trusted pointer.
+ * @dst: bpf_dynptr to buffer where to store the result. Must be a trusted pointer.
+ * @iv: bpf_dynptr to IV data to be used by decryptor.
+ *
+ * Encrypts provided buffer using IV data and the crypto context. Crypto context must be configured.
+ */
+__bpf_kfunc int bpf_crypto_skcipher_encrypt(struct bpf_crypto_skcipher_ctx *ctx,
+ const struct bpf_dynptr_kern *src,
+ struct bpf_dynptr_kern *dst,
+ struct bpf_dynptr_kern *iv)
+{
+ return bpf_crypto_skcipher_crypt(ctx->tfm, src, dst, iv, false);
+}
+
+__bpf_kfunc_end_defs();
+
+BTF_SET8_START(crypt_skcipher_init_kfunc_btf_ids)
+BTF_ID_FLAGS(func, bpf_crypto_skcipher_ctx_create, KF_ACQUIRE | KF_RET_NULL | KF_SLEEPABLE)
+BTF_ID_FLAGS(func, bpf_crypto_skcipher_ctx_release, KF_RELEASE)
+BTF_ID_FLAGS(func, bpf_crypto_skcipher_ctx_acquire, KF_ACQUIRE | KF_TRUSTED_ARGS)
+BTF_SET8_END(crypt_skcipher_init_kfunc_btf_ids)
+
+static const struct btf_kfunc_id_set crypt_skcipher_init_kfunc_set = {
+ .owner = THIS_MODULE,
+ .set = &crypt_skcipher_init_kfunc_btf_ids,
+};
+
+BTF_SET8_START(crypt_skcipher_kfunc_btf_ids)
+BTF_ID_FLAGS(func, bpf_crypto_skcipher_decrypt, KF_RCU)
+BTF_ID_FLAGS(func, bpf_crypto_skcipher_encrypt, KF_RCU)
+BTF_SET8_END(crypt_skcipher_kfunc_btf_ids)
+
+static const struct btf_kfunc_id_set crypt_skcipher_kfunc_set = {
+ .owner = THIS_MODULE,
+ .set = &crypt_skcipher_kfunc_btf_ids,
+};
+
+BTF_ID_LIST(crypto_skcipher_dtor_ids)
+BTF_ID(struct, bpf_crypto_skcipher_ctx)
+BTF_ID(func, bpf_crypto_skcipher_ctx_release)
+
+static int __init crypto_skcipher_kfunc_init(void)
+{
+ int ret;
+ const struct btf_id_dtor_kfunc crypto_skcipher_dtors[] = {
+ {
+ .btf_id = crypto_skcipher_dtor_ids[0],
+ .kfunc_btf_id = crypto_skcipher_dtor_ids[1]
+ },
+ };
+
+ ret = register_btf_kfunc_id_set(BPF_PROG_TYPE_SCHED_CLS, &crypt_skcipher_kfunc_set);
+ ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_SCHED_ACT, &crypt_skcipher_kfunc_set);
+ ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_XDP, &crypt_skcipher_kfunc_set);
+ ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_UNSPEC,
+ &crypt_skcipher_init_kfunc_set);
+ return ret ?: register_btf_id_dtor_kfuncs(crypto_skcipher_dtors,
+ ARRAY_SIZE(crypto_skcipher_dtors),
+ THIS_MODULE);
+}
+
+late_initcall(crypto_skcipher_kfunc_init);
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 03517db5cfb3..eb6ddc7e7fdc 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -1436,7 +1436,7 @@ static const struct bpf_func_proto bpf_kptr_xchg_proto = {
#define DYNPTR_SIZE_MASK 0xFFFFFF
#define DYNPTR_RDONLY_BIT BIT(31)
-static bool __bpf_dynptr_is_rdonly(const struct bpf_dynptr_kern *ptr)
+bool __bpf_dynptr_is_rdonly(const struct bpf_dynptr_kern *ptr)
{
return ptr->size & DYNPTR_RDONLY_BIT;
}
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 9ae6eae13471..aac46e7f22ab 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -5552,6 +5552,7 @@ BTF_ID(struct, cgroup)
#endif
BTF_ID(struct, bpf_cpumask)
BTF_ID(struct, task_struct)
+BTF_ID(struct, bpf_crypto_skcipher_ctx)
BTF_SET_END(rcu_protected_types)
static bool rcu_protected_object(const struct btf *btf, u32 btf_id)
--
2.39.3
^ permalink raw reply related
* [PATCH bpf-next v4 2/2] selftests: bpf: crypto skcipher algo selftests
From: Vadim Fedorenko @ 2023-11-10 20:35 UTC (permalink / raw)
To: Vadim Fedorenko, Jakub Kicinski, Martin KaFai Lau,
Andrii Nakryiko, Alexei Starovoitov, Mykola Lysenko
Cc: Vadim Fedorenko, bpf, netdev, linux-crypto
In-Reply-To: <20231110203500.2787316-1-vadfed@meta.com>
Add simple tc hook selftests to show the way to work with new crypto
BPF API. Some weird structre and map are added to setup program to make
verifier happy about dynptr initialization from memory. Simple AES-ECB
algo is used to demonstrate encryption and decryption of fixed size
buffers.
Signed-off-by: Vadim Fedorenko <vadfed@meta.com>
---
v3 -> v4:
- adjust selftests to use new syntax of helpers
- add tests for acquire and release
v2 -> v3:
- disable tests on s390 and aarch64 because of unknown Fatal exception
in sg_init_one
v1 -> v2:
- add CONFIG_CRYPTO_AES and CONFIG_CRYPTO_ECB to selftest build config
suggested by Daniel
---
tools/testing/selftests/bpf/DENYLIST.aarch64 | 1 +
tools/testing/selftests/bpf/DENYLIST.s390x | 1 +
tools/testing/selftests/bpf/config | 3 +
.../selftests/bpf/prog_tests/crypto_sanity.c | 148 ++++++++++++++
.../selftests/bpf/progs/crypto_common.h | 69 +++++++
.../selftests/bpf/progs/crypto_sanity.c | 193 ++++++++++++++++++
6 files changed, 415 insertions(+)
create mode 100644 tools/testing/selftests/bpf/prog_tests/crypto_sanity.c
create mode 100644 tools/testing/selftests/bpf/progs/crypto_common.h
create mode 100644 tools/testing/selftests/bpf/progs/crypto_sanity.c
diff --git a/tools/testing/selftests/bpf/DENYLIST.aarch64 b/tools/testing/selftests/bpf/DENYLIST.aarch64
index 5c2cc7e8c5d0..72c7ef3e5872 100644
--- a/tools/testing/selftests/bpf/DENYLIST.aarch64
+++ b/tools/testing/selftests/bpf/DENYLIST.aarch64
@@ -1,5 +1,6 @@
bpf_cookie/multi_kprobe_attach_api # kprobe_multi_link_api_subtest:FAIL:fentry_raw_skel_load unexpected error: -3
bpf_cookie/multi_kprobe_link_api # kprobe_multi_link_api_subtest:FAIL:fentry_raw_skel_load unexpected error: -3
+crypto_sanity # sg_init_one has exception on aarch64
exceptions # JIT does not support calling kfunc bpf_throw: -524
fexit_sleep # The test never returns. The remaining tests cannot start.
kprobe_multi_bench_attach # needs CONFIG_FPROBE
diff --git a/tools/testing/selftests/bpf/DENYLIST.s390x b/tools/testing/selftests/bpf/DENYLIST.s390x
index 1a63996c0304..8ab7485bedb1 100644
--- a/tools/testing/selftests/bpf/DENYLIST.s390x
+++ b/tools/testing/selftests/bpf/DENYLIST.s390x
@@ -1,5 +1,6 @@
# TEMPORARY
# Alphabetical order
+crypto_sanity # sg_init_one has exception on s390 (exceptions)
exceptions # JIT does not support calling kfunc bpf_throw (exceptions)
get_stack_raw_tp # user_stack corrupted user stack (no backchain userspace)
stacktrace_build_id # compare_map_keys stackid_hmap vs. stackmap err -2 errno 2 (?)
diff --git a/tools/testing/selftests/bpf/config b/tools/testing/selftests/bpf/config
index 3ec5927ec3e5..81e521e9c0e9 100644
--- a/tools/testing/selftests/bpf/config
+++ b/tools/testing/selftests/bpf/config
@@ -14,6 +14,9 @@ CONFIG_CGROUP_BPF=y
CONFIG_CRYPTO_HMAC=y
CONFIG_CRYPTO_SHA256=y
CONFIG_CRYPTO_USER_API_HASH=y
+CONFIG_CRYPTO_SKCIPHER=y
+CONFIG_CRYPTO_ECB=y
+CONFIG_CRYPTO_AES=y
CONFIG_DEBUG_INFO=y
CONFIG_DEBUG_INFO_BTF=y
CONFIG_DEBUG_INFO_DWARF4=y
diff --git a/tools/testing/selftests/bpf/prog_tests/crypto_sanity.c b/tools/testing/selftests/bpf/prog_tests/crypto_sanity.c
new file mode 100644
index 000000000000..eb2cf7677797
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/crypto_sanity.c
@@ -0,0 +1,148 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2023 Meta Platforms, Inc. and affiliates. */
+
+#include <sys/types.h>
+#include <sys/socket.h>
+#include <net/if.h>
+#include <linux/in6.h>
+
+#include "test_progs.h"
+#include "network_helpers.h"
+#include "crypto_sanity.skel.h"
+
+#define NS_TEST "crypto_sanity_ns"
+#define IPV6_IFACE_ADDR "face::1"
+#define UDP_TEST_PORT 7777
+static const char plain_text[] = "stringtoencrypt0";
+static const char crypted_data[] = "\x5B\x59\x39\xEA\xD9\x7A\x2D\xAD\xA7\xE0\x43" \
+ "\x37\x8A\x77\x17\xB2";
+
+void test_crypto_sanity(void)
+{
+ LIBBPF_OPTS(bpf_tc_hook, qdisc_hook, .attach_point = BPF_TC_EGRESS);
+ LIBBPF_OPTS(bpf_tc_opts, tc_attach_enc);
+ LIBBPF_OPTS(bpf_tc_opts, tc_attach_dec);
+ LIBBPF_OPTS(bpf_test_run_opts, opts,
+ .repeat = 1,
+ );
+ struct nstoken *nstoken = NULL;
+ struct crypto_sanity *skel;
+ struct sockaddr_in6 addr;
+ int sockfd, err, pfd;
+ socklen_t addrlen;
+
+ skel = crypto_sanity__open();
+ if (!ASSERT_OK_PTR(skel, "skel open"))
+ return;
+
+ bpf_program__set_autoload(skel->progs.crypto_accuire, true);
+
+ err = crypto_sanity__load(skel);
+ if (!ASSERT_ERR(err, "crypto_accuire unexpected load success"))
+ goto fail;
+
+ crypto_sanity__destroy(skel);
+
+ skel = crypto_sanity__open();
+ if (!ASSERT_OK_PTR(skel, "skel open"))
+ return;
+
+ bpf_program__set_autoload(skel->progs.crypto_accuire, false);
+
+ SYS(fail, "ip netns add %s", NS_TEST);
+ SYS(fail, "ip -net %s -6 addr add %s/128 dev lo nodad", NS_TEST, IPV6_IFACE_ADDR);
+ SYS(fail, "ip -net %s link set dev lo up", NS_TEST);
+
+ err = crypto_sanity__load(skel);
+ if (!ASSERT_OK(err, "crypto_sanity__load"))
+ goto fail;
+
+ nstoken = open_netns(NS_TEST);
+ if (!ASSERT_OK_PTR(nstoken, "open_netns"))
+ goto fail;
+
+ qdisc_hook.ifindex = if_nametoindex("lo");
+ if (!ASSERT_GT(qdisc_hook.ifindex, 0, "if_nametoindex lo"))
+ goto fail;
+
+ err = crypto_sanity__attach(skel);
+ if (!ASSERT_OK(err, "crypto_sanity__attach"))
+ goto fail;
+
+ pfd = bpf_program__fd(skel->progs.crypto_release);
+ if (!ASSERT_GT(pfd, 0, "crypto_release fd"))
+ goto fail;
+
+ err = bpf_prog_test_run_opts(pfd, &opts);
+ if (!ASSERT_OK(err, "crypto_release") ||
+ !ASSERT_OK(opts.retval, "crypto_release retval"))
+ goto fail;
+
+ pfd = bpf_program__fd(skel->progs.skb_crypto_setup);
+ if (!ASSERT_GT(pfd, 0, "skb_crypto_setup fd"))
+ goto fail;
+
+ err = bpf_prog_test_run_opts(pfd, &opts);
+ if (!ASSERT_OK(err, "skb_crypto_setup") ||
+ !ASSERT_OK(opts.retval, "skb_crypto_setup retval"))
+ goto fail;
+
+ if (!ASSERT_OK(skel->bss->status, "skb_crypto_setup status"))
+ goto fail;
+
+ err = bpf_tc_hook_create(&qdisc_hook);
+ if (!ASSERT_OK(err, "create qdisc hook"))
+ goto fail;
+
+ addrlen = sizeof(addr);
+ err = make_sockaddr(AF_INET6, IPV6_IFACE_ADDR, UDP_TEST_PORT,
+ (void *)&addr, &addrlen);
+ if (!ASSERT_OK(err, "make_sockaddr"))
+ goto fail;
+
+ tc_attach_dec.prog_fd = bpf_program__fd(skel->progs.decrypt_sanity);
+ err = bpf_tc_attach(&qdisc_hook, &tc_attach_dec);
+ if (!ASSERT_OK(err, "attach decrypt filter"))
+ goto fail;
+
+ sockfd = socket(AF_INET6, SOCK_DGRAM, 0);
+ if (!ASSERT_NEQ(sockfd, -1, "decrypt socket"))
+ goto fail;
+ err = sendto(sockfd, crypted_data, 16, 0, (void *)&addr, addrlen);
+ close(sockfd);
+ if (!ASSERT_EQ(err, 16, "decrypt send"))
+ goto fail;
+
+ bpf_tc_detach(&qdisc_hook, &tc_attach_dec);
+ if (!ASSERT_OK(skel->bss->status, "decrypt status"))
+ goto fail;
+ if (!ASSERT_STRNEQ(skel->bss->dst, plain_text, sizeof(plain_text), "decrypt"))
+ goto fail;
+
+ tc_attach_enc.prog_fd = bpf_program__fd(skel->progs.encrypt_sanity);
+ err = bpf_tc_attach(&qdisc_hook, &tc_attach_enc);
+ if (!ASSERT_OK(err, "attach encrypt filter"))
+ goto fail;
+
+ sockfd = socket(AF_INET6, SOCK_DGRAM, 0);
+ if (!ASSERT_NEQ(sockfd, -1, "encrypt socket"))
+ goto fail;
+ err = sendto(sockfd, plain_text, 16, 0, (void *)&addr, addrlen);
+ close(sockfd);
+ if (!ASSERT_EQ(err, 16, "encrypt send"))
+ goto fail;
+
+ bpf_tc_detach(&qdisc_hook, &tc_attach_enc);
+ if (!ASSERT_OK(skel->bss->status, "encrypt status"))
+ goto fail;
+ if (!ASSERT_STRNEQ(skel->bss->dst, crypted_data, sizeof(crypted_data), "encrypt"))
+ goto fail;
+
+fail:
+ if (nstoken) {
+ bpf_tc_hook_destroy(&qdisc_hook);
+ close_netns(nstoken);
+ }
+ SYS_NOFAIL("ip netns del " NS_TEST " &> /dev/null");
+ crypto_sanity__destroy(skel);
+}
diff --git a/tools/testing/selftests/bpf/progs/crypto_common.h b/tools/testing/selftests/bpf/progs/crypto_common.h
new file mode 100644
index 000000000000..6874bc7b84dd
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/crypto_common.h
@@ -0,0 +1,69 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (c) 2023 Meta Platforms, Inc. and affiliates. */
+
+#ifndef _CRYPTO_COMMON_H
+#define _CRYPTO_COMMON_H
+
+#include "errno.h"
+#include <stdbool.h>
+
+struct bpf_crypto_skcipher_ctx *bpf_crypto_skcipher_ctx_create(const char *algo__str,
+ const struct bpf_dynptr *key,
+ int *err) __ksym;
+struct bpf_crypto_skcipher_ctx *bpf_crypto_skcipher_ctx_acquire(struct bpf_crypto_skcipher_ctx *ctx) __ksym;
+void bpf_crypto_skcipher_ctx_release(struct bpf_crypto_skcipher_ctx *ctx) __ksym;
+int bpf_crypto_skcipher_encrypt(struct bpf_crypto_skcipher_ctx *ctx,
+ const struct bpf_dynptr *src, struct bpf_dynptr *dst,
+ struct bpf_dynptr *iv) __ksym;
+int bpf_crypto_skcipher_decrypt(struct bpf_crypto_skcipher_ctx *ctx,
+ const struct bpf_dynptr *src, struct bpf_dynptr *dst,
+ struct bpf_dynptr *iv) __ksym;
+
+struct __crypto_skcipher_ctx_value {
+ struct bpf_crypto_skcipher_ctx __kptr * ctx;
+};
+
+struct array_map {
+ __uint(type, BPF_MAP_TYPE_ARRAY);
+ __type(key, int);
+ __type(value, struct __crypto_skcipher_ctx_value);
+ __uint(max_entries, 1);
+} __crypto_skcipher_ctx_map SEC(".maps");
+
+static inline struct __crypto_skcipher_ctx_value *crypto_skcipher_ctx_value_lookup(void)
+{
+ u32 key = 0;
+
+ return bpf_map_lookup_elem(&__crypto_skcipher_ctx_map, &key);
+}
+
+static inline int crypto_skcipher_ctx_insert(struct bpf_crypto_skcipher_ctx *ctx)
+{
+ struct __crypto_skcipher_ctx_value local, *v;
+ struct bpf_crypto_skcipher_ctx *old;
+ u32 key = 0;
+ int err;
+
+ local.ctx = NULL;
+ err = bpf_map_update_elem(&__crypto_skcipher_ctx_map, &key, &local, 0);
+ if (err) {
+ bpf_crypto_skcipher_ctx_release(ctx);
+ return err;
+ }
+
+ v = bpf_map_lookup_elem(&__crypto_skcipher_ctx_map, &key);
+ if (!v) {
+ bpf_crypto_skcipher_ctx_release(ctx);
+ return -ENOENT;
+ }
+
+ old = bpf_kptr_xchg(&v->ctx, ctx);
+ if (old) {
+ bpf_crypto_skcipher_ctx_release(old);
+ return -EEXIST;
+ }
+
+ return 0;
+}
+
+#endif /* _CRYPTO_COMMON_H */
diff --git a/tools/testing/selftests/bpf/progs/crypto_sanity.c b/tools/testing/selftests/bpf/progs/crypto_sanity.c
new file mode 100644
index 000000000000..26fe9aff8b74
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/crypto_sanity.c
@@ -0,0 +1,193 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2023 Meta Platforms, Inc. and affiliates. */
+
+#include "vmlinux.h"
+#include "bpf_tracing_net.h"
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_endian.h>
+#include <bpf/bpf_tracing.h>
+#include "bpf_misc.h"
+#include "bpf_kfuncs.h"
+#include "crypto_common.h"
+
+#define UDP_TEST_PORT 7777
+unsigned char crypto_key[16] = "testtest12345678";
+const char crypto_algo[9] = "ecb(aes)";
+char dst[32] = {};
+int status;
+
+static int skb_dynptr_validate(struct __sk_buff *skb, struct bpf_dynptr *psrc)
+{
+ struct ipv6hdr ip6h;
+ struct udphdr udph;
+ u32 offset;
+
+ if (skb->protocol != __bpf_constant_htons(ETH_P_IPV6))
+ return -1;
+
+ if (bpf_skb_load_bytes(skb, ETH_HLEN, &ip6h, sizeof(ip6h)))
+ return -1;
+
+ if (ip6h.nexthdr != IPPROTO_UDP)
+ return -1;
+
+ if (bpf_skb_load_bytes(skb, ETH_HLEN + sizeof(ip6h), &udph, sizeof(udph)))
+ return -1;
+
+ if (udph.dest != __bpf_constant_htons(UDP_TEST_PORT))
+ return -1;
+
+ offset = ETH_HLEN + sizeof(ip6h) + sizeof(udph);
+ if (skb->len < offset + 16)
+ return -1;
+
+ bpf_dynptr_from_skb(skb, 0, psrc);
+ bpf_dynptr_adjust(psrc, offset, offset + 16);
+
+ return 0;
+}
+
+SEC("fentry.s/bpf_fentry_test1")
+int BPF_PROG(skb_crypto_setup)
+{
+ struct bpf_crypto_skcipher_ctx *cctx;
+ struct bpf_dynptr key = {};
+ int err = 0;
+
+ status = 0;
+
+ bpf_dynptr_from_mem(crypto_key, sizeof(crypto_key), 0, &key);
+ cctx = bpf_crypto_skcipher_ctx_create(crypto_algo, &key, &err);
+
+ if (!cctx) {
+ status = err;
+ return 0;
+ }
+
+ err = crypto_skcipher_ctx_insert(cctx);
+ if (err && err != -EEXIST)
+ status = err;
+
+ return 0;
+}
+
+SEC("fentry.s/bpf_fentry_test1")
+int BPF_PROG(crypto_release)
+{
+ struct bpf_crypto_skcipher_ctx *cctx;
+ struct bpf_dynptr key = {};
+ int err = 0;
+
+ status = 0;
+
+ bpf_dynptr_from_mem(crypto_key, sizeof(crypto_key), 0, &key);
+ cctx = bpf_crypto_skcipher_ctx_create(crypto_algo, &key, &err);
+
+ if (!cctx) {
+ status = err;
+ return 0;
+ }
+
+ bpf_crypto_skcipher_ctx_release(cctx);
+
+ return 0;
+}
+
+SEC("?fentry.s/bpf_fentry_test1")
+__failure __msg("Unreleased reference")
+int BPF_PROG(crypto_accuire)
+{
+ struct bpf_crypto_skcipher_ctx *cctx;
+ struct bpf_dynptr key = {};
+ int err = 0;
+
+ status = 0;
+
+ bpf_dynptr_from_mem(crypto_key, sizeof(crypto_key), 0, &key);
+ cctx = bpf_crypto_skcipher_ctx_create(crypto_algo, &key, &err);
+
+ if (!cctx) {
+ status = err;
+ return 0;
+ }
+
+ cctx = bpf_crypto_skcipher_ctx_acquire(cctx);
+ if (!cctx)
+ return -EINVAL;
+
+ bpf_crypto_skcipher_ctx_release(cctx);
+
+ return 0;
+}
+
+SEC("tc")
+int decrypt_sanity(struct __sk_buff *skb)
+{
+ struct __crypto_skcipher_ctx_value *v;
+ struct bpf_crypto_skcipher_ctx *ctx;
+ struct bpf_dynptr psrc, pdst, iv;
+ int err;
+
+ err = skb_dynptr_validate(skb, &psrc);
+ if (err < 0) {
+ status = err;
+ return TC_ACT_SHOT;
+ }
+
+ v = crypto_skcipher_ctx_value_lookup();
+ if (!v) {
+ status = -ENOENT;
+ return TC_ACT_SHOT;
+ }
+
+ ctx = v->ctx;
+ if (!ctx) {
+ status = -ENOENT;
+ return TC_ACT_SHOT;
+ }
+
+ bpf_dynptr_from_mem(dst, sizeof(dst), 0, &pdst);
+ bpf_dynptr_from_mem(dst, 0, 0, &iv);
+
+ status = bpf_crypto_skcipher_decrypt(ctx, &psrc, &pdst, &iv);
+
+ return TC_ACT_SHOT;
+}
+
+SEC("tc")
+int encrypt_sanity(struct __sk_buff *skb)
+{
+ struct __crypto_skcipher_ctx_value *v;
+ struct bpf_crypto_skcipher_ctx *ctx;
+ struct bpf_dynptr psrc, pdst, iv;
+ int err;
+
+ status = 0;
+
+ err = skb_dynptr_validate(skb, &psrc);
+ if (err < 0) {
+ status = err;
+ return TC_ACT_SHOT;
+ }
+
+ v = crypto_skcipher_ctx_value_lookup();
+ if (!v) {
+ status = -ENOENT;
+ return TC_ACT_SHOT;
+ }
+
+ ctx = v->ctx;
+ if (!ctx) {
+ status = -ENOENT;
+ return TC_ACT_SHOT;
+ }
+
+ bpf_dynptr_from_mem(dst, sizeof(dst), 0, &pdst);
+ bpf_dynptr_from_mem(dst, 0, 0, &iv);
+
+ status = bpf_crypto_skcipher_encrypt(ctx, &psrc, &pdst, &iv);
+
+ return TC_ACT_SHOT;
+}
+
+char __license[] SEC("license") = "GPL";
--
2.39.3
^ permalink raw reply related
* Re: [PATCH net] gve: Fixes for napi_poll when budget is 0
From: Jakub Kicinski @ 2023-11-10 20:36 UTC (permalink / raw)
To: Ziwei Xiao; +Cc: netdev, davem
In-Reply-To: <20231109235916.1105860-1-ziweixiao@google.com>
On Thu, 9 Nov 2023 15:59:16 -0800 Ziwei Xiao wrote:
> Fixes: f5cedc84a30d ("gve: Add transmit and receive support")
You need to CC everyone who put their tag on that patch. Use:
./scripts/get_maintainer.pl --git-min-percent 25 0001-your.patch
> diff --git a/drivers/net/ethernet/google/gve/gve_main.c b/drivers/net/ethernet/google/gve/gve_main.c
> index 276f996f95dc..5a84ccfd3423 100644
> --- a/drivers/net/ethernet/google/gve/gve_main.c
> +++ b/drivers/net/ethernet/google/gve/gve_main.c
> @@ -254,16 +254,16 @@ static int gve_napi_poll(struct napi_struct *napi, int budget)
> if (block->tx) {
> if (block->tx->q_num < priv->tx_cfg.num_queues)
> reschedule |= gve_tx_poll(block, budget);
> - else
> + else if (budget)
So here you use it as a bool
> reschedule |= gve_xdp_poll(block, budget);
> }
>
> - if (block->rx) {
> + if (block->rx && budget > 0) {
here as a signed int
> work_done = gve_rx_poll(block, budget);
> reschedule |= work_done == budget;
> }
>
> - if (reschedule)
> + if (reschedule || budget == 0)
and here you compare to 0
Why is every single condition different :S
Just add a new if, before the block->rx handling which does:
if (!budget)
return 0;
--
pw-bot: cr
pv-bot: cc
^ 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