* [PATCH net-next 11/11] selftests: mlxsw: Check device info on activated line card
From: Ido Schimmel @ 2022-04-25 3:44 UTC (permalink / raw)
To: netdev
Cc: davem, kuba, pabeni, jiri, petrm, dsahern, andrew, mlxsw,
Ido Schimmel
In-Reply-To: <20220425034431.3161260-1-idosch@nvidia.com>
From: Jiri Pirko <jiri@nvidia.com>
Once line card is activated, check the device FW version is exposed.
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
.../drivers/net/mlxsw/devlink_linecard.sh | 22 +++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/tools/testing/selftests/drivers/net/mlxsw/devlink_linecard.sh b/tools/testing/selftests/drivers/net/mlxsw/devlink_linecard.sh
index 04bedd98eb8b..53a65f416770 100755
--- a/tools/testing/selftests/drivers/net/mlxsw/devlink_linecard.sh
+++ b/tools/testing/selftests/drivers/net/mlxsw/devlink_linecard.sh
@@ -259,6 +259,26 @@ interface_check()
setup_wait
}
+lc_devices_info_check()
+{
+ local lc=$1
+ local expected_device_count=$2
+ local device_count
+ local device
+ local running_device_fw
+
+ device_count=$(devlink lc info $DEVLINK_DEV lc $lc -j | \
+ jq -e -r ".[][][].devices |length")
+ check_err $? "Failed to get linecard $lc device count"
+ for (( device=0; device<device_count; device++ ))
+ do
+ running_device_fw=$(devlink lc -v info $DEVLINK_DEV lc $lc -j | \
+ jq -e -r ".[][][].devices[$device].versions.running.fw")
+ check_err $? "Failed to get linecard $lc device $device running fw version"
+ log_info "Linecard $lc device $device running.fw: \"$running_device_fw\""
+ done
+}
+
activation_16x100G_test()
{
RET=0
@@ -275,6 +295,8 @@ activation_16x100G_test()
$ACTIVATION_TIMEOUT)
check_err $? "Failed to get linecard $lc activated (timeout)"
+ lc_devices_info_check $lc $LC_16X100G_DEVICE_COUNT
+
interface_check
log_test "Activation 16x100G"
--
2.33.1
^ permalink raw reply related
* Re: [PATCH net] tcp: fix potential xmit stalls caused by TCP_NOTSENT_LOWAT
From: Soheil Hassas Yeganeh @ 2022-04-25 4:06 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, netdev,
Eric Dumazet, Doug Porter, Neal Cardwell
In-Reply-To: <20220425003407.3002429-1-eric.dumazet@gmail.com>
On Sun, Apr 24, 2022 at 8:34 PM Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
> From: Eric Dumazet <edumazet@google.com>
>
> I had this bug sitting for too long in my pile, it is time to fix it.
>
> Thanks to Doug Porter for reminding me of it!
>
> We had various attempts in the past, including commit
> 0cbe6a8f089e ("tcp: remove SOCK_QUEUE_SHRUNK"),
> but the issue is that TCP stack currently only generates
> EPOLLOUT from input path, when tp->snd_una has advanced
> and skb(s) cleaned from rtx queue.
>
> If a flow has a big RTT, and/or receives SACKs, it is possible
> that the notsent part (tp->write_seq - tp->snd_nxt) reaches 0
> and no more data can be sent until tp->snd_una finally advances.
>
> What is needed is to also check if POLLOUT needs to be generated
> whenever tp->snd_nxt is advanced, from output path.
>
> This bug triggers more often after an idle period, as
> we do not receive ACK for at least one RTT. tcp_notsent_lowat
> could be a fraction of what CWND and pacing rate would allow to
> send during this RTT.
>
> In a followup patch, I will remove the bogus call
> to tcp_chrono_stop(sk, TCP_CHRONO_SNDBUF_LIMITED)
> from tcp_check_space(). Fact that we have decided to generate
> an EPOLLOUT does not mean the application has immediately
> refilled the transmit queue. This optimistic call
> might have been the reason the bug seemed not too serious.
>
> Tested:
>
> 200 ms rtt, 1% packet loss, 32 MB tcp_rmem[2] and tcp_wmem[2]
>
> $ echo 500000 >/proc/sys/net/ipv4/tcp_notsent_lowat
> $ cat bench_rr.sh
> SUM=0
> for i in {1..10}
> do
> V=`netperf -H remote_host -l30 -t TCP_RR -- -r 10000000,10000 -o LOCAL_BYTES_SENT | egrep -v "MIGRATED|Bytes"`
> echo $V
> SUM=$(($SUM + $V))
> done
> echo SUM=$SUM
>
> Before patch:
> $ bench_rr.sh
> 130000000
> 80000000
> 140000000
> 140000000
> 140000000
> 140000000
> 130000000
> 40000000
> 90000000
> 110000000
> SUM=1140000000
>
> After patch:
> $ bench_rr.sh
> 430000000
> 590000000
> 530000000
> 450000000
> 450000000
> 350000000
> 450000000
> 490000000
> 480000000
> 460000000
> SUM=4680000000 # This is 410 % of the value before patch.
>
> Fixes: c9bee3b7fdec ("tcp: TCP_NOTSENT_LOWAT socket option")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: Doug Porter <dsp@fb.com>
> Cc: Soheil Hassas Yeganeh <soheil@google.com>
> Cc: Neal Cardwell <ncardwell@google.com>
Acked-by: Soheil Hassas Yeganeh <soheil@google.com>
Thank you for the fix!
> ---
> include/net/tcp.h | 1 +
> net/ipv4/tcp_input.c | 12 +++++++++++-
> net/ipv4/tcp_output.c | 1 +
> 3 files changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index 9987b3fba9f202632916cc439af9d17f1e68bcd3..cc1295037533a7741e454f7c040f77a21deae02b 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -621,6 +621,7 @@ void tcp_synack_rtt_meas(struct sock *sk, struct request_sock *req);
> void tcp_reset(struct sock *sk, struct sk_buff *skb);
> void tcp_skb_mark_lost_uncond_verify(struct tcp_sock *tp, struct sk_buff *skb);
> void tcp_fin(struct sock *sk);
> +void tcp_check_space(struct sock *sk);
>
> /* tcp_timer.c */
> void tcp_init_xmit_timers(struct sock *);
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index 2088f93fa37b5fb9110e7933242a27bd4009990e..48f6075228600896daa6507c4cd06acfc851a0fa 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -5454,7 +5454,17 @@ static void tcp_new_space(struct sock *sk)
> INDIRECT_CALL_1(sk->sk_write_space, sk_stream_write_space, sk);
> }
>
> -static void tcp_check_space(struct sock *sk)
> +/* Caller made space either from:
> + * 1) Freeing skbs in rtx queues (after tp->snd_una has advanced)
> + * 2) Sent skbs from output queue (and thus advancing tp->snd_nxt)
> + *
> + * We might be able to generate EPOLLOUT to the application if:
> + * 1) Space consumed in output/rtx queues is below sk->sk_sndbuf/2
> + * 2) notsent amount (tp->write_seq - tp->snd_nxt) became
> + * small enough that tcp_stream_memory_free() decides it
> + * is time to generate EPOLLOUT.
> + */
> +void tcp_check_space(struct sock *sk)
> {
> /* pairs with tcp_poll() */
> smp_mb();
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index 9ede847f4199844c5884e3f62ea450562072a0a7..1ca2f28c9981018e6cfaee3435d711467af6048d 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -82,6 +82,7 @@ static void tcp_event_new_data_sent(struct sock *sk, struct sk_buff *skb)
>
> NET_ADD_STATS(sock_net(sk), LINUX_MIB_TCPORIGDATASENT,
> tcp_skb_pcount(skb));
> + tcp_check_space(sk);
> }
>
> /* SND.NXT, if window was not shrunk or the amount of shrunk was less than one
> --
> 2.36.0.rc2.479.g8af0fa9b8e-goog
>
^ permalink raw reply
* [PATCH net] drivers: net: can: Fix deadlock in grcan_close()
From: Duoming Zhou @ 2022-04-25 4:24 UTC (permalink / raw)
To: linux-kernel
Cc: wg, mkl, davem, kuba, pabeni, linux-can, netdev, Duoming Zhou
There are deadlocks caused by del_timer_sync(&priv->hang_timer)
and del_timer_sync(&priv->rr_timer) in grcan_close(), one of
the deadlocks are shown below:
(Thread 1) | (Thread 2)
| grcan_reset_timer()
grcan_close() | mod_timer()
spin_lock_irqsave() //(1) | (wait a time)
... | grcan_initiate_running_reset()
del_timer_sync() | spin_lock_irqsave() //(2)
(wait timer to stop) | ...
We hold priv->lock in position (1) of thread 1 and use
del_timer_sync() to wait timer to stop, but timer handler
also need priv->lock in position (2) of thread 2.
As a result, grcan_close() will block forever.
This patch extracts del_timer_sync() from the protection of
spin_lock_irqsave(), which could let timer handler to obtain
the needed lock.
Signed-off-by: Duoming Zhou <duoming@zju.edu.cn>
---
drivers/net/can/grcan.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/can/grcan.c b/drivers/net/can/grcan.c
index d0c5a7a60da..1189057b5d6 100644
--- a/drivers/net/can/grcan.c
+++ b/drivers/net/can/grcan.c
@@ -1102,8 +1102,10 @@ static int grcan_close(struct net_device *dev)
priv->closing = true;
if (priv->need_txbug_workaround) {
+ spin_unlock_irqrestore(&priv->lock, flags);
del_timer_sync(&priv->hang_timer);
del_timer_sync(&priv->rr_timer);
+ spin_lock_irqsave(&priv->lock, flags);
}
netif_stop_queue(dev);
grcan_stop_hardware(dev);
--
2.17.1
^ permalink raw reply related
* [PATCH net-next RESEND] net: phy: marvell-88x2222: set proper phydev->port
From: Ivan Bornyakov @ 2022-04-25 4:16 UTC (permalink / raw)
To: i.bornyakov
Cc: Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
Jakub Kicinski, Paolo Abeni, netdev, linux-kernel, system
In-Reply-To: <20220405150305.151573-1-i.bornyakov@metrotek.ru>
phydev->port was not set and always reported as PORT_TP.
Set phydev->port according to inserted SFP module.
Signed-off-by: Ivan Bornyakov <i.bornyakov@metrotek.ru>
---
drivers/net/phy/marvell-88x2222.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/phy/marvell-88x2222.c b/drivers/net/phy/marvell-88x2222.c
index ec4f1407a78c..9f971b37ec35 100644
--- a/drivers/net/phy/marvell-88x2222.c
+++ b/drivers/net/phy/marvell-88x2222.c
@@ -603,6 +603,7 @@ static int mv2222_sfp_insert(void *upstream, const struct sfp_eeprom_id *id)
dev = &phydev->mdio.dev;
sfp_parse_support(phydev->sfp_bus, id, sfp_supported);
+ phydev->port = sfp_parse_port(phydev->sfp_bus, id, sfp_supported);
sfp_interface = sfp_select_interface(phydev->sfp_bus, sfp_supported);
dev_info(dev, "%s SFP module inserted\n", phy_modes(sfp_interface));
@@ -639,6 +640,7 @@ static void mv2222_sfp_remove(void *upstream)
priv->line_interface = PHY_INTERFACE_MODE_NA;
linkmode_zero(priv->supported);
+ phydev->port = PORT_OTHER;
}
static void mv2222_sfp_link_up(void *upstream)
--
2.25.1
^ permalink raw reply related
* Re: [PATCH] add support for Sierra Wireless EM7590 0xc081 composition.
From: Bjørn Mork @ 2022-04-25 5:07 UTC (permalink / raw)
To: Ethan Yang
Cc: davem, kuba, pabeni, netdev, linux-usb, linux-kernel, gchiang,
Ethan Yang
In-Reply-To: <20220425031411.4030-1-etyang@sierrawireless.com>
Ethan Yang <ipis.yang@gmail.com> writes:
> add support for Sierra Wireless EM7590 0xc081 composition.
>
> Signed-off-by: Ethan Yang <etyang@sierrawireless.com>
> ---
> drivers/net/usb/qmi_wwan.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
> index 3353e761016d..fa220a13edb6 100644
> --- a/drivers/net/usb/qmi_wwan.c
> +++ b/drivers/net/usb/qmi_wwan.c
> @@ -1351,6 +1351,7 @@ static const struct usb_device_id products[] = {
> {QMI_QUIRK_SET_DTR(0x1199, 0x907b, 8)}, /* Sierra Wireless EM74xx */
> {QMI_QUIRK_SET_DTR(0x1199, 0x907b, 10)},/* Sierra Wireless EM74xx */
> {QMI_QUIRK_SET_DTR(0x1199, 0x9091, 8)}, /* Sierra Wireless EM7565 */
> + {QMI_QUIRK_SET_DTR(0x1199, 0xc081, 8)}, /* Sierra Wireless EM7590 */
> {QMI_FIXED_INTF(0x1bbb, 0x011e, 4)}, /* Telekom Speedstick LTE II (Alcatel One Touch L100V LTE) */
> {QMI_FIXED_INTF(0x1bbb, 0x0203, 2)}, /* Alcatel L800MA */
> {QMI_FIXED_INTF(0x2357, 0x0201, 4)}, /* TP-LINK HSUPA Modem MA180 */
Hello!
The diff looks fine to me, but there are some minor issues with the
commit message. This is how it ended up looking in for me with "git
log":
---
commit 5e3f386fdb23c8e5ba7c24ff5efd6beb73db3e85 (HEAD -> bugfix-master)
Author: Ethan Yang <ipis.yang@gmail.com>
Date: Mon Apr 25 11:14:11 2022 +0800
add support for Sierra Wireless EM7590 0xc081 composition.
add support for Sierra Wireless EM7590 0xc081 composition.
Signed-off-by: Ethan Yang <etyang@sierrawireless.com>
---
The issues I see there are:
1) subject should have a prefix - e.g. "net: usb: qmi_wwan:"
2) don't repeat the subject in the body - that's redundant and ends up
looking like a duplicate line in the commit message. Write
something addding more information instead. Anything you find
relevant
3) I guess you send from a gmail address for a reason. But you most
likely want the Author field to match your Signed-off-by? You can fix
this by including something like this as the *first* line of the
message body:
From: Ethan Yang <etyang@sierrawireless.com>
FYI: checkpatch warns about the last issue:
bjorn@miraculix:/usr/local/src/git/linux$ scripts/checkpatch.pl /tmp/xx
WARNING: From:/Signed-off-by: email address mismatch: 'From: Ethan Yang <ipis.yang@gmail.com>' != 'Signed-off-by: Ethan Yang <etyang@sierrawireless.com>'
total: 0 errors, 1 warnings, 0 checks, 7 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
/tmp/xx has style problems, please review.
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
Bjørn
^ permalink raw reply
* [PATCH v2] net: usb: qmi_wwan: add support for Sierra Wireless EM7590
From: ipis.yang @ 2022-04-25 5:40 UTC (permalink / raw)
To: bjorn, davem, kuba, pabeni, netdev, linux-usb, linux-kernel
Cc: gchiang, ipis.yang, Ethan Yang
In-Reply-To: <87bkwpkayv.fsf@miraculix.mork.no>
From: Ethan Yang <etyang@sierrawireless.com>
add support for Sierra Wireless EM7590 0xc081 composition.
Signed-off-by: Ethan Yang <etyang@sierrawireless.com>
---
drivers/net/usb/qmi_wwan.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index 3353e761016d..fa220a13edb6 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -1351,6 +1351,7 @@ static const struct usb_device_id products[] = {
{QMI_QUIRK_SET_DTR(0x1199, 0x907b, 8)}, /* Sierra Wireless EM74xx */
{QMI_QUIRK_SET_DTR(0x1199, 0x907b, 10)},/* Sierra Wireless EM74xx */
{QMI_QUIRK_SET_DTR(0x1199, 0x9091, 8)}, /* Sierra Wireless EM7565 */
+ {QMI_QUIRK_SET_DTR(0x1199, 0xc081, 8)}, /* Sierra Wireless EM7590 */
{QMI_FIXED_INTF(0x1bbb, 0x011e, 4)}, /* Telekom Speedstick LTE II (Alcatel One Touch L100V LTE) */
{QMI_FIXED_INTF(0x1bbb, 0x0203, 2)}, /* Alcatel L800MA */
{QMI_FIXED_INTF(0x2357, 0x0201, 4)}, /* TP-LINK HSUPA Modem MA180 */
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v2] net: usb: qmi_wwan: add support for Sierra Wireless EM7590
From: Bjørn Mork @ 2022-04-25 5:43 UTC (permalink / raw)
To: ipis.yang
Cc: davem, kuba, pabeni, netdev, linux-usb, linux-kernel, gchiang,
Ethan Yang
In-Reply-To: <20220425054028.5444-1-etyang@sierrawireless.com>
Thanks!
Acked-by: Bjørn Mork <bjorn@mork.no>
^ permalink raw reply
* Re: [PATCH v2] mediatek/mt7601u: add debugfs exit function
From: Kalle Valo @ 2022-04-25 6:01 UTC (permalink / raw)
To: z
Cc: Jakub Kicinski, David S. Miller, Paolo Abeni, Matthias Brugger,
linux-wireless, netdev, linux-arm-kernel, linux-mediatek,
linux-kernel, bernard
In-Reply-To: <15b4f.2aad.1805ece06a1.Coremail.zhaojunkui2008@126.com>
z <zhaojunkui2008@126.com> writes:
> At 2022-04-23 03:47:04, "Jakub Kicinski" <kubakici@wp.pl> wrote:
>>On Fri, 22 Apr 2022 01:08:54 -0700 Bernard Zhao wrote:
>>> When mt7601u loaded, there are two cases:
>>> First when mt7601u is loaded, in function mt7601u_probe, if
>>> function mt7601u_probe run into error lable err_hw,
>>> mt7601u_cleanup didn`t cleanup the debugfs node.
>>> Second when the module disconnect, in function mt7601u_disconnect,
>>> mt7601u_cleanup didn`t cleanup the debugfs node.
>>> This patch add debugfs exit function and try to cleanup debugfs
>>> node when mt7601u loaded fail or unloaded.
>>>
>>> Signed-off-by: Bernard Zhao <zhaojunkui2008@126.com>
>>
>>Ah, missed that there was a v2. My point stands, wiphy debugfs dir
>>should do the cleanup.
>>
>>Do you encounter problems in practice or are you sending this patches
>>based on reading / static analysis of the code only.
>
> Hi Jakub Kicinski:
>
> The issue here is found by reading code.
> I read the drivers/net/wireless code and found that many modules are
> not cleanup the debugfs.
> I sorted out the modules that were not cleaned up the debugfs:
> ./ti/wl18xx
> ./ti/wl12xx
> ./intel/iwlwifi
> ./intel/iwlwifi
> ./mediatek/mt76
> I am not sure whether this part is welcome to kernel so I submitted a patch.
> If you have any suggestions, welcome to put forward for discussion, thank you！
Jakub is saying that wiphy_unregister() recursively removes the debugfs
directories:
/*
* First remove the hardware from everywhere, this makes
* it impossible to find from userspace.
*/
debugfs_remove_recursive(rdev->wiphy.debugfsdir);
So AFAICS there is no bug. But if you are testing this on a real
hardware and something is wrong, please provide more info.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH net-next v3 1/2] rtnetlink: add extack support in fdb del handlers
From: Julia Lawall @ 2022-04-25 6:11 UTC (permalink / raw)
To: Alaa Mohamed
Cc: Nikolay Aleksandrov, netdev, outreachy, roopa, jdenham, sbrivio,
jesse.brandeburg, anthony.l.nguyen, davem, kuba, pabeni,
vladimir.oltean, claudiu.manoil, alexandre.belloni, shshaikh,
manishc, intel-wired-lan, linux-kernel, UNGLinuxDriver,
GR-Linux-NIC-Dev, bridge
In-Reply-To: <3bcb2d3d-8b8b-8a8f-1285-7277394b4e6b@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 11714 bytes --]
On Sun, 24 Apr 2022, Alaa Mohamed wrote:
>
> On ٢٤/٤/٢٠٢٢ ٢١:٥٥, Nikolay Aleksandrov wrote:
> > On 24/04/2022 22:49, Alaa Mohamed wrote:
> > > On ٢٤/٤/٢٠٢٢ ٢١:٠٢, Nikolay Aleksandrov wrote:
> > > > On 24/04/2022 15:09, Alaa Mohamed wrote:
> > > > > Add extack support to .ndo_fdb_del in netdevice.h and
> > > > > all related methods.
> > > > >
> > > > > Signed-off-by: Alaa Mohamed <eng.alaamohamedsoliman.am@gmail.com>
> > > > > ---
> > > > > changes in V3:
> > > > > fix errors reported by checkpatch.pl
> > > > > ---
> > > > > drivers/net/ethernet/intel/ice/ice_main.c | 4 ++--
> > > > > drivers/net/ethernet/mscc/ocelot_net.c | 4 ++--
> > > > > drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c | 2 +-
> > > > > drivers/net/macvlan.c | 2 +-
> > > > > drivers/net/vxlan/vxlan_core.c | 2 +-
> > > > > include/linux/netdevice.h | 2 +-
> > > > > net/bridge/br_fdb.c | 2 +-
> > > > > net/bridge/br_private.h | 2 +-
> > > > > net/core/rtnetlink.c | 4 ++--
> > > > > 9 files changed, 12 insertions(+), 12 deletions(-)
> > > > >
> > > > > diff --git a/drivers/net/ethernet/intel/ice/ice_main.c
> > > > > b/drivers/net/ethernet/intel/ice/ice_main.c
> > > > > index d768925785ca..7b55d8d94803 100644
> > > > > --- a/drivers/net/ethernet/intel/ice/ice_main.c
> > > > > +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> > > > > @@ -5678,10 +5678,10 @@ ice_fdb_add(struct ndmsg *ndm, struct nlattr
> > > > > __always_unused *tb[],
> > > > > static int
> > > > > ice_fdb_del(struct ndmsg *ndm, __always_unused struct nlattr *tb[],
> > > > > struct net_device *dev, const unsigned char *addr,
> > > > > - __always_unused u16 vid)
> > > > > + __always_unused u16 vid, struct netlink_ext_ack *extack)
> > > > > {
> > > > > int err;
> > > > > -
> > > > > +
> > > > What's changed here?
> > > In the previous version, I removed the blank line after "int err;" and you
> > > said I shouldn't so I added blank line.
> > >
> > Yeah, my question is are you fixing a dos ending or something else?
> > The blank line is already there, what's wrong with it?
> No, I didn't.
OK, so what is the answer to the question about what changed? It looks
like you remove a blank line and then add it back. But that should not
show up as a difference when you generate the patch.
When you answer a comment, please put a blank line before and after your
answer. Otherwise it can be hard to see your answer when it is in the
middle of a larger patch.
> >
> > The point is it's not nice to mix style fixes and other changes, more so
> > if nothing is mentioned in the commit message.
> Got it, So, what should I do to fix it?
A series? But it is not clear that any change is needed here at all.
julia
> > > > > if (ndm->ndm_state & NUD_PERMANENT) {
> > > > > netdev_err(dev, "FDB only supports static addresses\n");
> > > > > return -EINVAL;
> > > > > diff --git a/drivers/net/ethernet/mscc/ocelot_net.c
> > > > > b/drivers/net/ethernet/mscc/ocelot_net.c
> > > > > index 247bc105bdd2..e07c64e3159c 100644
> > > > > --- a/drivers/net/ethernet/mscc/ocelot_net.c
> > > > > +++ b/drivers/net/ethernet/mscc/ocelot_net.c
> > > > > @@ -774,14 +774,14 @@ static int ocelot_port_fdb_add(struct ndmsg
> > > > > *ndm, struct nlattr *tb[],
> > > > >
> > > > > static int ocelot_port_fdb_del(struct ndmsg *ndm, struct nlattr
> > > > > *tb[],
> > > > > struct net_device *dev,
> > > > > - const unsigned char *addr, u16 vid)
> > > > > + const unsigned char *addr, u16 vid, struct
> > > > > netlink_ext_ack *extack)
> > > > > {
> > > > > struct ocelot_port_private *priv = netdev_priv(dev);
> > > > > struct ocelot_port *ocelot_port = &priv->port;
> > > > > struct ocelot *ocelot = ocelot_port->ocelot;
> > > > > int port = priv->chip_port;
> > > > >
> > > > > - return ocelot_fdb_del(ocelot, port, addr, vid,
> > > > > ocelot_port->bridge);
> > > > > + return ocelot_fdb_del(ocelot, port, addr, vid,
> > > > > ocelot_port->bridge, extack);
> > > > > }
> > > > >
> > > > > static int ocelot_port_fdb_dump(struct sk_buff *skb,
> > > > > diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
> > > > > b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
> > > > > index d320567b2cca..51fa23418f6a 100644
> > > > > --- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
> > > > > +++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
> > > > > @@ -368,7 +368,7 @@ static int qlcnic_set_mac(struct net_device
> > > > > *netdev, void *p)
> > > > >
> > > > > static int qlcnic_fdb_del(struct ndmsg *ndm, struct nlattr *tb[],
> > > > > struct net_device *netdev,
> > > > > - const unsigned char *addr, u16 vid)
> > > > > + const unsigned char *addr, u16 vid, struct
> > > > > netlink_ext_ack *extack)
> > > > > {
> > > > > struct qlcnic_adapter *adapter = netdev_priv(netdev);
> > > > > int err = -EOPNOTSUPP;
> > > > > diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
> > > > > index 069e8824c264..ffd34d9f7049 100644
> > > > > --- a/drivers/net/macvlan.c
> > > > > +++ b/drivers/net/macvlan.c
> > > > > @@ -1017,7 +1017,7 @@ static int macvlan_fdb_add(struct ndmsg *ndm,
> > > > > struct nlattr *tb[],
> > > > >
> > > > > static int macvlan_fdb_del(struct ndmsg *ndm, struct nlattr *tb[],
> > > > > struct net_device *dev,
> > > > > - const unsigned char *addr, u16 vid)
> > > > > + const unsigned char *addr, u16 vid, struct
> > > > > netlink_ext_ack *extack)
> > > > > {
> > > > > struct macvlan_dev *vlan = netdev_priv(dev);
> > > > > int err = -EINVAL;
> > > > > diff --git a/drivers/net/vxlan/vxlan_core.c
> > > > > b/drivers/net/vxlan/vxlan_core.c
> > > > > index de97ff98d36e..cf2f60037340 100644
> > > > > --- a/drivers/net/vxlan/vxlan_core.c
> > > > > +++ b/drivers/net/vxlan/vxlan_core.c
> > > > > @@ -1280,7 +1280,7 @@ int __vxlan_fdb_delete(struct vxlan_dev *vxlan,
> > > > > /* Delete entry (via netlink) */
> > > > > static int vxlan_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
> > > > > struct net_device *dev,
> > > > > - const unsigned char *addr, u16 vid)
> > > > > + const unsigned char *addr, u16 vid, struct
> > > > > netlink_ext_ack *extack)
> > > > > {
> > > > > struct vxlan_dev *vxlan = netdev_priv(dev);
> > > > > union vxlan_addr ip;
> > > > > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> > > > > index 28ea4f8269d4..d0d2a8f33c73 100644
> > > > > --- a/include/linux/netdevice.h
> > > > > +++ b/include/linux/netdevice.h
> > > > > @@ -1509,7 +1509,7 @@ struct net_device_ops {
> > > > > struct nlattr *tb[],
> > > > > struct net_device *dev,
> > > > > const unsigned char *addr,
> > > > > - u16 vid);
> > > > > + u16 vid, struct netlink_ext_ack *extack);
> > > > > int (*ndo_fdb_dump)(struct sk_buff *skb,
> > > > > struct netlink_callback *cb,
> > > > > struct net_device *dev,
> > > > > diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
> > > > > index 6ccda68bd473..5bfce2e9a553 100644
> > > > > --- a/net/bridge/br_fdb.c
> > > > > +++ b/net/bridge/br_fdb.c
> > > > > @@ -1110,7 +1110,7 @@ static int __br_fdb_delete(struct net_bridge
> > > > > *br,
> > > > > /* Remove neighbor entry with RTM_DELNEIGH */
> > > > > int br_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
> > > > > struct net_device *dev,
> > > > > - const unsigned char *addr, u16 vid)
> > > > > + const unsigned char *addr, u16 vid, struct netlink_ext_ack
> > > > > *extack)
> > > > > {
> > > > > struct net_bridge_vlan_group *vg;
> > > > > struct net_bridge_port *p = NULL;
> > > > > diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> > > > > index 18ccc3d5d296..95348c1c9ce5 100644
> > > > > --- a/net/bridge/br_private.h
> > > > > +++ b/net/bridge/br_private.h
> > > > > @@ -780,7 +780,7 @@ void br_fdb_update(struct net_bridge *br, struct
> > > > > net_bridge_port *source,
> > > > > const unsigned char *addr, u16 vid, unsigned long
> > > > > flags);
> > > > >
> > > > > int br_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
> > > > > - struct net_device *dev, const unsigned char *addr, u16
> > > > > vid);
> > > > > + struct net_device *dev, const unsigned char *addr, u16 vid,
> > > > > struct netlink_ext_ack *extack);
> > > > This is way too long (111 chars) and checkpatch should've complained
> > > > about it.
> > > > WARNING: line length of 111 exceeds 100 columns
> > > > #234: FILE: net/bridge/br_private.h:782:
> > > > + struct net_device *dev, const unsigned char *addr, u16 vid,
> > > > struct netlink_ext_ack *extack);
> > > I will fix it.
> > >
> > > > > int br_fdb_add(struct ndmsg *nlh, struct nlattr *tb[], struct
> > > > > net_device *dev,
> > > > > const unsigned char *addr, u16 vid, u16 nlh_flags,
> > > > > struct netlink_ext_ack *extack);
> > > > > diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> > > > > index 4041b3e2e8ec..99b30ae58a47 100644
> > > > > --- a/net/core/rtnetlink.c
> > > > > +++ b/net/core/rtnetlink.c
> > > > > @@ -4223,7 +4223,7 @@ static int rtnl_fdb_del(struct sk_buff *skb,
> > > > > struct nlmsghdr *nlh,
> > > > > const struct net_device_ops *ops = br_dev->netdev_ops;
> > > > >
> > > > > if (ops->ndo_fdb_del)
> > > > > - err = ops->ndo_fdb_del(ndm, tb, dev, addr, vid);
> > > > > + err = ops->ndo_fdb_del(ndm, tb, dev, addr, vid, extack);
> > > > >
> > > > > if (err)
> > > > > goto out;
> > > > > @@ -4235,7 +4235,7 @@ static int rtnl_fdb_del(struct sk_buff *skb,
> > > > > struct nlmsghdr *nlh,
> > > > > if (ndm->ndm_flags & NTF_SELF) {
> > > > > if (dev->netdev_ops->ndo_fdb_del)
> > > > > err = dev->netdev_ops->ndo_fdb_del(ndm, tb, dev, addr,
> > > > > - vid);
> > > > > + vid, extack);
> > > > > else
> > > > > err = ndo_dflt_fdb_del(ndm, tb, dev, addr, vid);
> > > > >
> > > > > --
> > > > > 2.36.0
> > > > >
>
>
^ permalink raw reply
* Re: [PATCH net-next v3 1/2] rtnetlink: add extack support in fdb del handlers
From: Julia Lawall @ 2022-04-25 6:12 UTC (permalink / raw)
To: Alaa Mohamed
Cc: Nikolay Aleksandrov, netdev, outreachy, roopa, jdenham, sbrivio,
jesse.brandeburg, anthony.l.nguyen, davem, kuba, pabeni,
vladimir.oltean, claudiu.manoil, alexandre.belloni, shshaikh,
manishc, intel-wired-lan, linux-kernel, UNGLinuxDriver,
GR-Linux-NIC-Dev, bridge
In-Reply-To: <d89eefc2-664f-8537-d10e-6fdfbb6823ed@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 8678 bytes --]
On Sun, 24 Apr 2022, Alaa Mohamed wrote:
>
> On ٢٤/٤/٢٠٢٢ ٢١:٠٢, Nikolay Aleksandrov wrote:
> > On 24/04/2022 15:09, Alaa Mohamed wrote:
> > > Add extack support to .ndo_fdb_del in netdevice.h and
> > > all related methods.
> > >
> > > Signed-off-by: Alaa Mohamed <eng.alaamohamedsoliman.am@gmail.com>
> > > ---
> > > changes in V3:
> > > fix errors reported by checkpatch.pl
> > > ---
> > > drivers/net/ethernet/intel/ice/ice_main.c | 4 ++--
> > > drivers/net/ethernet/mscc/ocelot_net.c | 4 ++--
> > > drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c | 2 +-
> > > drivers/net/macvlan.c | 2 +-
> > > drivers/net/vxlan/vxlan_core.c | 2 +-
> > > include/linux/netdevice.h | 2 +-
> > > net/bridge/br_fdb.c | 2 +-
> > > net/bridge/br_private.h | 2 +-
> > > net/core/rtnetlink.c | 4 ++--
> > > 9 files changed, 12 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/drivers/net/ethernet/intel/ice/ice_main.c
> > > b/drivers/net/ethernet/intel/ice/ice_main.c
> > > index d768925785ca..7b55d8d94803 100644
> > > --- a/drivers/net/ethernet/intel/ice/ice_main.c
> > > +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> > > @@ -5678,10 +5678,10 @@ ice_fdb_add(struct ndmsg *ndm, struct nlattr
> > > __always_unused *tb[],
> > > static int
> > > ice_fdb_del(struct ndmsg *ndm, __always_unused struct nlattr *tb[],
> > > struct net_device *dev, const unsigned char *addr,
> > > - __always_unused u16 vid)
> > > + __always_unused u16 vid, struct netlink_ext_ack *extack)
> > > {
> > > int err;
> > > -
> > > +
> > What's changed here?
>
> In the previous version, I removed the blank line after "int err;" and you
> said I shouldn't so I added blank line.
This answer doesn't make sense. You should make a patch against the code
as it is in the kernel, not against your previous proposal.
julia
>
> >
> > > if (ndm->ndm_state & NUD_PERMANENT) {
> > > netdev_err(dev, "FDB only supports static addresses\n");
> > > return -EINVAL;
> > > diff --git a/drivers/net/ethernet/mscc/ocelot_net.c
> > > b/drivers/net/ethernet/mscc/ocelot_net.c
> > > index 247bc105bdd2..e07c64e3159c 100644
> > > --- a/drivers/net/ethernet/mscc/ocelot_net.c
> > > +++ b/drivers/net/ethernet/mscc/ocelot_net.c
> > > @@ -774,14 +774,14 @@ static int ocelot_port_fdb_add(struct ndmsg *ndm,
> > > struct nlattr *tb[],
> > >
> > > static int ocelot_port_fdb_del(struct ndmsg *ndm, struct nlattr *tb[],
> > > struct net_device *dev,
> > > - const unsigned char *addr, u16 vid)
> > > + const unsigned char *addr, u16 vid, struct
> > > netlink_ext_ack *extack)
> > > {
> > > struct ocelot_port_private *priv = netdev_priv(dev);
> > > struct ocelot_port *ocelot_port = &priv->port;
> > > struct ocelot *ocelot = ocelot_port->ocelot;
> > > int port = priv->chip_port;
> > >
> > > - return ocelot_fdb_del(ocelot, port, addr, vid, ocelot_port->bridge);
> > > + return ocelot_fdb_del(ocelot, port, addr, vid, ocelot_port->bridge,
> > > extack);
> > > }
> > >
> > > static int ocelot_port_fdb_dump(struct sk_buff *skb,
> > > diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
> > > b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
> > > index d320567b2cca..51fa23418f6a 100644
> > > --- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
> > > +++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
> > > @@ -368,7 +368,7 @@ static int qlcnic_set_mac(struct net_device *netdev,
> > > void *p)
> > >
> > > static int qlcnic_fdb_del(struct ndmsg *ndm, struct nlattr *tb[],
> > > struct net_device *netdev,
> > > - const unsigned char *addr, u16 vid)
> > > + const unsigned char *addr, u16 vid, struct
> > > netlink_ext_ack *extack)
> > > {
> > > struct qlcnic_adapter *adapter = netdev_priv(netdev);
> > > int err = -EOPNOTSUPP;
> > > diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
> > > index 069e8824c264..ffd34d9f7049 100644
> > > --- a/drivers/net/macvlan.c
> > > +++ b/drivers/net/macvlan.c
> > > @@ -1017,7 +1017,7 @@ static int macvlan_fdb_add(struct ndmsg *ndm, struct
> > > nlattr *tb[],
> > >
> > > static int macvlan_fdb_del(struct ndmsg *ndm, struct nlattr *tb[],
> > > struct net_device *dev,
> > > - const unsigned char *addr, u16 vid)
> > > + const unsigned char *addr, u16 vid, struct
> > > netlink_ext_ack *extack)
> > > {
> > > struct macvlan_dev *vlan = netdev_priv(dev);
> > > int err = -EINVAL;
> > > diff --git a/drivers/net/vxlan/vxlan_core.c
> > > b/drivers/net/vxlan/vxlan_core.c
> > > index de97ff98d36e..cf2f60037340 100644
> > > --- a/drivers/net/vxlan/vxlan_core.c
> > > +++ b/drivers/net/vxlan/vxlan_core.c
> > > @@ -1280,7 +1280,7 @@ int __vxlan_fdb_delete(struct vxlan_dev *vxlan,
> > > /* Delete entry (via netlink) */
> > > static int vxlan_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
> > > struct net_device *dev,
> > > - const unsigned char *addr, u16 vid)
> > > + const unsigned char *addr, u16 vid, struct
> > > netlink_ext_ack *extack)
> > > {
> > > struct vxlan_dev *vxlan = netdev_priv(dev);
> > > union vxlan_addr ip;
> > > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> > > index 28ea4f8269d4..d0d2a8f33c73 100644
> > > --- a/include/linux/netdevice.h
> > > +++ b/include/linux/netdevice.h
> > > @@ -1509,7 +1509,7 @@ struct net_device_ops {
> > > struct nlattr *tb[],
> > > struct net_device *dev,
> > > const unsigned char *addr,
> > > - u16 vid);
> > > + u16 vid, struct netlink_ext_ack
> > > *extack);
> > > int (*ndo_fdb_dump)(struct sk_buff *skb,
> > > struct netlink_callback *cb,
> > > struct net_device *dev,
> > > diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
> > > index 6ccda68bd473..5bfce2e9a553 100644
> > > --- a/net/bridge/br_fdb.c
> > > +++ b/net/bridge/br_fdb.c
> > > @@ -1110,7 +1110,7 @@ static int __br_fdb_delete(struct net_bridge *br,
> > > /* Remove neighbor entry with RTM_DELNEIGH */
> > > int br_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
> > > struct net_device *dev,
> > > - const unsigned char *addr, u16 vid)
> > > + const unsigned char *addr, u16 vid, struct netlink_ext_ack
> > > *extack)
> > > {
> > > struct net_bridge_vlan_group *vg;
> > > struct net_bridge_port *p = NULL;
> > > diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> > > index 18ccc3d5d296..95348c1c9ce5 100644
> > > --- a/net/bridge/br_private.h
> > > +++ b/net/bridge/br_private.h
> > > @@ -780,7 +780,7 @@ void br_fdb_update(struct net_bridge *br, struct
> > > net_bridge_port *source,
> > > const unsigned char *addr, u16 vid, unsigned long flags);
> > >
> > > int br_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
> > > - struct net_device *dev, const unsigned char *addr, u16 vid);
> > > + struct net_device *dev, const unsigned char *addr, u16 vid,
> > > struct netlink_ext_ack *extack);
> > This is way too long (111 chars) and checkpatch should've complained about
> > it.
> > WARNING: line length of 111 exceeds 100 columns
> > #234: FILE: net/bridge/br_private.h:782:
> > + struct net_device *dev, const unsigned char *addr, u16 vid,
> > struct netlink_ext_ack *extack);
>
> I will fix it.
>
> >
> > > int br_fdb_add(struct ndmsg *nlh, struct nlattr *tb[], struct net_device
> > > *dev,
> > > const unsigned char *addr, u16 vid, u16 nlh_flags,
> > > struct netlink_ext_ack *extack);
> > > diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> > > index 4041b3e2e8ec..99b30ae58a47 100644
> > > --- a/net/core/rtnetlink.c
> > > +++ b/net/core/rtnetlink.c
> > > @@ -4223,7 +4223,7 @@ static int rtnl_fdb_del(struct sk_buff *skb, struct
> > > nlmsghdr *nlh,
> > > const struct net_device_ops *ops = br_dev->netdev_ops;
> > >
> > > if (ops->ndo_fdb_del)
> > > - err = ops->ndo_fdb_del(ndm, tb, dev, addr, vid);
> > > + err = ops->ndo_fdb_del(ndm, tb, dev, addr, vid,
> > > extack);
> > >
> > > if (err)
> > > goto out;
> > > @@ -4235,7 +4235,7 @@ static int rtnl_fdb_del(struct sk_buff *skb, struct
> > > nlmsghdr *nlh,
> > > if (ndm->ndm_flags & NTF_SELF) {
> > > if (dev->netdev_ops->ndo_fdb_del)
> > > err = dev->netdev_ops->ndo_fdb_del(ndm, tb, dev, addr,
> > > - vid);
> > > + vid, extack);
> > > else
> > > err = ndo_dflt_fdb_del(ndm, tb, dev, addr, vid);
> > >
> > > --
> > > 2.36.0
> > >
>
>
^ permalink raw reply
* Re: [PATCH] ath10k: skip ath10k_halt during suspend for driver state RESTARTING
From: Kalle Valo @ 2022-04-25 6:14 UTC (permalink / raw)
To: Abhishek Kumar
Cc: linux-kernel, linux-wireless, briannorris, ath10k, netdev,
Wen Gong, David S. Miller, Jakub Kicinski, Paolo Abeni
In-Reply-To: <20220425021442.1.I650b809482e1af8d0156ed88b5dc2677a0711d46@changeid>
Abhishek Kumar <kuabhs@chromium.org> writes:
> Double free crash is observed when FW recovery(caused by wmi
> timeout/crash) is followed by immediate suspend event. The FW recovery
> is triggered by ath10k_core_restart() which calls driver clean up via
> ath10k_halt(). When the suspend event occurs between the FW recovery,
> the restart worker thread is put into frozen state until suspend completes.
> The suspend event triggers ath10k_stop() which again triggers ath10k_halt()
> The double invocation of ath10k_halt() causes ath10k_htt_rx_free() to be
> called twice(Note: ath10k_htt_rx_alloc was not called by restart worker
> thread because of its frozen state), causing the crash.
>
> To fix this, during the suspend flow, skip call to ath10k_halt() in
> ath10k_stop() when the current driver state is ATH10K_STATE_RESTARTING.
> Also, for driver state ATH10K_STATE_RESTARTING, call
> ath10k_wait_for_suspend() in ath10k_stop(). This is because call to
> ath10k_wait_for_suspend() is skipped later in
> [ath10k_halt() > ath10k_core_stop()] for the driver state
> ATH10K_STATE_RESTARTING.
>
> The frozen restart worker thread will be cancelled during resume when the
> device comes out of suspend.
>
> Below is the crash stack for reference:
>
> [ 428.469167] ------------[ cut here ]------------
> [ 428.469180] kernel BUG at mm/slub.c:4150!
> [ 428.469193] invalid opcode: 0000 [#1] PREEMPT SMP NOPTI
> [ 428.469219] Workqueue: events_unbound async_run_entry_fn
> [ 428.469230] RIP: 0010:kfree+0x319/0x31b
> [ 428.469241] RSP: 0018:ffffa1fac015fc30 EFLAGS: 00010246
> [ 428.469247] RAX: ffffedb10419d108 RBX: ffff8c05262b0000
> [ 428.469252] RDX: ffff8c04a8c07000 RSI: 0000000000000000
> [ 428.469256] RBP: ffffa1fac015fc78 R08: 0000000000000000
> [ 428.469276] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 428.469285] Call Trace:
> [ 428.469295] ? dma_free_attrs+0x5f/0x7d
> [ 428.469320] ath10k_core_stop+0x5b/0x6f
> [ 428.469336] ath10k_halt+0x126/0x177
> [ 428.469352] ath10k_stop+0x41/0x7e
> [ 428.469387] drv_stop+0x88/0x10e
> [ 428.469410] __ieee80211_suspend+0x297/0x411
> [ 428.469441] rdev_suspend+0x6e/0xd0
> [ 428.469462] wiphy_suspend+0xb1/0x105
> [ 428.469483] ? name_show+0x2d/0x2d
> [ 428.469490] dpm_run_callback+0x8c/0x126
> [ 428.469511] ? name_show+0x2d/0x2d
> [ 428.469517] __device_suspend+0x2e7/0x41b
> [ 428.469523] async_suspend+0x1f/0x93
> [ 428.469529] async_run_entry_fn+0x3d/0xd1
> [ 428.469535] process_one_work+0x1b1/0x329
> [ 428.469541] worker_thread+0x213/0x372
> [ 428.469547] kthread+0x150/0x15f
> [ 428.469552] ? pr_cont_work+0x58/0x58
> [ 428.469558] ? kthread_blkcg+0x31/0x31
>
> Signed-off-by: Abhishek Kumar <kuabhs@chromium.org>
> Co-developed-by: Wen Gong <quic_wgong@quicinc.com>
> Signed-off-by: Wen Gong <quic_wgong@quicinc.com>
Tested-on tag missing, but I can add it if you provide it.
https://wireless.wiki.kernel.org/en/users/drivers/ath10k/submittingpatches#tested-on_tag
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCHv2 net-next] net/af_packet: add VLAN support for AF_PACKET SOCK_RAW GSO
From: Hangbin Liu @ 2022-04-25 6:21 UTC (permalink / raw)
To: netdev
Cc: Michael S . Tsirkin, Jason Wang, David S . Miller, Jakub Kicinski,
Paolo Abeni, Maxim Mikityanskiy, Willem de Bruijn, Balazs Nemeth,
Mike Pattrick, Eric Dumazet
In-Reply-To: <20220425014502.985464-1-liuhangbin@gmail.com>
Cc Willem as I didn't format the address correctly.
On Mon, Apr 25, 2022 at 09:45:02AM +0800, Hangbin Liu wrote:
> Currently, the kernel drops GSO VLAN tagged packet if it's created with
> socket(AF_PACKET, SOCK_RAW, 0) plus virtio_net_hdr.
>
> The reason is AF_PACKET doesn't adjust the skb network header if there is
> a VLAN tag. Then after virtio_net_hdr_set_proto() called, the skb->protocol
> will be set to ETH_P_IP/IPv6. And in later inet/ipv6_gso_segment() the skb
> is dropped as network header position is invalid.
>
> Let's handle VLAN packets by adjusting network header position in
> packet_parse_headers(). The adjustment is safe and does not affect the
> later xmit as tap device also did that.
>
> In packet_snd(), packet_parse_headers() need to be moved before calling
> virtio_net_hdr_set_proto(), so we can set correct skb->protocol and
> network header first.
>
> There is no need to update tpacket_snd() as it calls packet_parse_headers()
> in tpacket_fill_skb(), which is already before calling virtio_net_hdr_*
> functions.
>
> skb->no_fcs setting is also moved upper to make all skb settings together
> and keep consistency with function packet_sendmsg_spkt().
>
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> ---
>
> v2: Rewrite commit description, no code update.
> ---
> net/packet/af_packet.c | 18 +++++++++++++-----
> 1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> index 243566129784..fd31334cf688 100644
> --- a/net/packet/af_packet.c
> +++ b/net/packet/af_packet.c
> @@ -1924,12 +1924,20 @@ static int packet_rcv_spkt(struct sk_buff *skb, struct net_device *dev,
>
> static void packet_parse_headers(struct sk_buff *skb, struct socket *sock)
> {
> + int depth;
> +
> if ((!skb->protocol || skb->protocol == htons(ETH_P_ALL)) &&
> sock->type == SOCK_RAW) {
> skb_reset_mac_header(skb);
> skb->protocol = dev_parse_header_protocol(skb);
> }
>
> + /* Move network header to the right position for VLAN tagged packets */
> + if (likely(skb->dev->type == ARPHRD_ETHER) &&
> + eth_type_vlan(skb->protocol) &&
> + __vlan_get_protocol(skb, skb->protocol, &depth) != 0)
> + skb_set_network_header(skb, depth);
> +
> skb_probe_transport_header(skb);
> }
>
> @@ -3047,6 +3055,11 @@ static int packet_snd(struct socket *sock, struct msghdr *msg, size_t len)
> skb->mark = sockc.mark;
> skb->tstamp = sockc.transmit_time;
>
> + if (unlikely(extra_len == 4))
> + skb->no_fcs = 1;
> +
> + packet_parse_headers(skb, sock);
> +
> if (has_vnet_hdr) {
> err = virtio_net_hdr_to_skb(skb, &vnet_hdr, vio_le());
> if (err)
> @@ -3055,11 +3068,6 @@ static int packet_snd(struct socket *sock, struct msghdr *msg, size_t len)
> virtio_net_hdr_set_proto(skb, &vnet_hdr);
> }
>
> - packet_parse_headers(skb, sock);
> -
> - if (unlikely(extra_len == 4))
> - skb->no_fcs = 1;
> -
> err = po->xmit(skb);
> if (unlikely(err != 0)) {
> if (err > 0)
> --
> 2.35.1
>
^ permalink raw reply
* [PATCH] net: phy: marvell10g: fix return value on error
From: Baruch Siach @ 2022-04-25 6:27 UTC (permalink / raw)
To: Russell King, Marek Behún; +Cc: netdev, Baruch Siach
From: Baruch Siach <baruch.siach@siklu.com>
Return back the error value that we get from phy_read_mmd().
Fixes: c84786fa8f91 ("net: phy: marvell10g: read copper results from CSSR1")
Signed-off-by: Baruch Siach <baruch.siach@siklu.com>
---
drivers/net/phy/marvell10g.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c
index b6fea119fe13..2b7d0720720b 100644
--- a/drivers/net/phy/marvell10g.c
+++ b/drivers/net/phy/marvell10g.c
@@ -880,7 +880,7 @@ static int mv3310_read_status_copper(struct phy_device *phydev)
cssr1 = phy_read_mmd(phydev, MDIO_MMD_PCS, MV_PCS_CSSR1);
if (cssr1 < 0)
- return val;
+ return cssr1;
/* If the link settings are not resolved, mark the link down */
if (!(cssr1 & MV_PCS_CSSR1_RESOLVED)) {
--
2.35.1
^ permalink raw reply related
* [PATCH v3] net: dsa: mv88e6xxx: Fix port_hidden_wait to account for port_base_addr
From: Nathan Rossi @ 2022-04-25 7:04 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: Nathan Rossi, Andrew Lunn, Vivien Didelot, Florian Fainelli,
Vladimir Oltean, David S. Miller, Jakub Kicinski, Paolo Abeni,
Marek Behún
The other port_hidden functions rely on the port_read/port_write
functions to access the hidden control port. These functions apply the
offset for port_base_addr where applicable. Update port_hidden_wait to
use the port_wait_bit so that port_base_addr offsets are accounted for
when waiting for the busy bit to change.
Without the offset the port_hidden_wait function would timeout on
devices that have a non-zero port_base_addr (e.g. MV88E6141), however
devices that have a zero port_base_addr would operate correctly (e.g.
MV88E6390).
Fixes: 609070133aff ("net: dsa: mv88e6xxx: update code operating on hidden registers")
Signed-off-by: Nathan Rossi <nathan@nathanrossi.com>
Reviewed-by: Marek Behún <kabel@kernel.org>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
Changes in v2:
- Add Fixes
Changes in v3:
- Changed Fixes to more recent commit where the bug is present
---
drivers/net/dsa/mv88e6xxx/port_hidden.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/port_hidden.c b/drivers/net/dsa/mv88e6xxx/port_hidden.c
index b49d05f0e1..7a9f9ff6de 100644
--- a/drivers/net/dsa/mv88e6xxx/port_hidden.c
+++ b/drivers/net/dsa/mv88e6xxx/port_hidden.c
@@ -40,8 +40,9 @@ int mv88e6xxx_port_hidden_wait(struct mv88e6xxx_chip *chip)
{
int bit = __bf_shf(MV88E6XXX_PORT_RESERVED_1A_BUSY);
- return mv88e6xxx_wait_bit(chip, MV88E6XXX_PORT_RESERVED_1A_CTRL_PORT,
- MV88E6XXX_PORT_RESERVED_1A, bit, 0);
+ return mv88e6xxx_port_wait_bit(chip,
+ MV88E6XXX_PORT_RESERVED_1A_CTRL_PORT,
+ MV88E6XXX_PORT_RESERVED_1A, bit, 0);
}
int mv88e6xxx_port_hidden_read(struct mv88e6xxx_chip *chip, int block, int port,
---
2.35.2
^ permalink raw reply related
* Re: [PATCH net-next 08/10] tls: rx: use async as an in-out argument
From: Gal Pressman @ 2022-04-25 7:19 UTC (permalink / raw)
To: Jakub Kicinski, davem, pabeni
Cc: netdev, borisp, john.fastabend, daniel, vfedorenko
In-Reply-To: <20220411191917.1240155-9-kuba@kernel.org>
On 11/04/2022 22:19, Jakub Kicinski wrote:
> Propagating EINPROGRESS thru multiple layers of functions is
> error prone. Use darg->async as an in/out argument, like we
> use darg->zc today. On input it tells the code if async is
> allowed, on output if it took place.
>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
I know this is not much to go on, but this patch broke our tls workflows
when device offload is enabled.
I'm still looking into it, but maybe you have an idea what might have
went wrong?
^ permalink raw reply
* Re: [PATCH net-next 03/28] sfc: Copy shared files needed for Siena
From: Martin Habets @ 2022-04-25 7:22 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: pabeni, davem, netdev, ecree.xilinx
In-Reply-To: <20220423065007.7a103878@kernel.org>
On Sat, Apr 23, 2022 at 06:50:07AM -0700, Jakub Kicinski wrote:
> On Fri, 22 Apr 2022 15:57:43 +0100 Martin Habets wrote:
> > From: Martin Habets <martinh@xilinx.com>
> >
> > No changes are done, those will be done with subsequent commits.
>
> This ginormous patch does not make it thru the mail systems.
> I'm guessing there is a (perfectly reasonable) 1MB limit somewhere.
I think the issue is with mcdi_pcol.h, which is 1.1MB of defines
generated from the hardware databases. It has grown slowely over the
years.
I'll split up this patch and see if I can manually cut down mcdi_pcol.h.
> I think you can also rework the series and combine the pure rename
> patches. Having the renames by header file does not substantially
> help review.
Ok, will do. I do not want to make individual patches too big.
> Try to stay under the 15 patch limit.
I'll probably need 2 or 3 series, and it means our Siena NICs will
not work after the 1st series.
Thanks for your advice,
Martin
^ permalink raw reply
* Re: [PATCH perf/core 3/5] perf tools: Move libbpf init in libbpf_init function
From: Jiri Olsa @ 2022-04-25 7:25 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
linux-perf-users, netdev, bpf, Ingo Molnar, Namhyung Kim,
Alexander Shishkin, Peter Zijlstra, Martin KaFai Lau, Song Liu,
Yonghong Song, John Fastabend, Ian Rogers
In-Reply-To: <YmLf3PQ9ws2C/Myu@kernel.org>
On Fri, Apr 22, 2022 at 02:03:24PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Fri, Apr 22, 2022 at 12:00:23PM +0200, Jiri Olsa escreveu:
> > Moving the libbpf init code into single function,
> > so we have single place doing that.
>
> Cherry picked this one, waiting for Andrii to chime in wrt the libbpf
> changes, if its acceptable, how to proceed, i.e. in what tree to carry
> these?
I think at this point it's ok with either yours perf/core or bpf-next/master,
I waited for them to be in sync for this ;-)
but as you pointed out there's issue with perf linked with dynamic libbpf,
because the current version does not have the libbpf_register_prog_handler
api that perf needs now.. also it needs the fix and api introduced in this
patchset
I'll check and perhaps we can temporirly disable perf/bpf prologue generation
for dynamic linking..? until the libbpf release has all the needed changes
jirka
>
> - Arnaldo
>
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> > tools/perf/util/bpf-loader.c | 27 ++++++++++++++++++---------
> > 1 file changed, 18 insertions(+), 9 deletions(-)
> >
> > diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c
> > index b72cef1ae959..f8ad581ea247 100644
> > --- a/tools/perf/util/bpf-loader.c
> > +++ b/tools/perf/util/bpf-loader.c
> > @@ -99,16 +99,26 @@ static int bpf_perf_object__add(struct bpf_object *obj)
> > return perf_obj ? 0 : -ENOMEM;
> > }
> >
> > +static int libbpf_init(void)
> > +{
> > + if (libbpf_initialized)
> > + return 0;
> > +
> > + libbpf_set_print(libbpf_perf_print);
> > + libbpf_initialized = true;
> > + return 0;
> > +}
> > +
> > struct bpf_object *
> > bpf__prepare_load_buffer(void *obj_buf, size_t obj_buf_sz, const char *name)
> > {
> > LIBBPF_OPTS(bpf_object_open_opts, opts, .object_name = name);
> > struct bpf_object *obj;
> > + int err;
> >
> > - if (!libbpf_initialized) {
> > - libbpf_set_print(libbpf_perf_print);
> > - libbpf_initialized = true;
> > - }
> > + err = libbpf_init();
> > + if (err)
> > + return ERR_PTR(err);
> >
> > obj = bpf_object__open_mem(obj_buf, obj_buf_sz, &opts);
> > if (IS_ERR_OR_NULL(obj)) {
> > @@ -135,14 +145,13 @@ struct bpf_object *bpf__prepare_load(const char *filename, bool source)
> > {
> > LIBBPF_OPTS(bpf_object_open_opts, opts, .object_name = filename);
> > struct bpf_object *obj;
> > + int err;
> >
> > - if (!libbpf_initialized) {
> > - libbpf_set_print(libbpf_perf_print);
> > - libbpf_initialized = true;
> > - }
> > + err = libbpf_init();
> > + if (err)
> > + return ERR_PTR(err);
> >
> > if (source) {
> > - int err;
> > void *obj_buf;
> > size_t obj_buf_sz;
> >
> > --
> > 2.35.1
>
> --
>
> - Arnaldo
^ permalink raw reply
* [syzbot] INFO: task hung in tls_sw_sendmsg (3)
From: syzbot @ 2022-04-25 7:30 UTC (permalink / raw)
To: andrii, ast, borisp, bpf, daniel, davem, john.fastabend, kafai,
kpsingh, kuba, linux-kernel, netdev, pabeni, songliubraving,
syzkaller-bugs, yhs
Hello,
syzbot found the following issue on:
HEAD commit: b253435746d9 Merge tag 'xtensa-20220416' of https://github..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=109cf862f00000
kernel config: https://syzkaller.appspot.com/x/.config?x=4cdc9619f45633df
dashboard link: https://syzkaller.appspot.com/bug?extid=baad3750d52fcc56930b
compiler: Debian clang version 13.0.1-++20220126092033+75e33f71c2da-1~exp1~20220126212112.63, GNU ld (GNU Binutils for Debian) 2.35.2
Unfortunately, I don't have any reproducer for this issue yet.
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+baad3750d52fcc56930b@syzkaller.appspotmail.com
INFO: task syz-executor.3:5663 blocked for more than 143 seconds.
Not tainted 5.18.0-rc3-syzkaller-00016-gb253435746d9 #0
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
task:syz-executor.3 state:D stack:28856 pid: 5663 ppid: 3623 flags:0x00004004
Call Trace:
<TASK>
context_switch kernel/sched/core.c:5073 [inline]
__schedule+0x939/0xea0 kernel/sched/core.c:6388
schedule+0xeb/0x1b0 kernel/sched/core.c:6460
schedule_preempt_disabled+0xf/0x20 kernel/sched/core.c:6519
__mutex_lock_common+0xecf/0x26e0 kernel/locking/mutex.c:673
__mutex_lock kernel/locking/mutex.c:733 [inline]
mutex_lock_nested+0x17/0x20 kernel/locking/mutex.c:785
tls_sw_sendmsg+0x297/0x1830 net/tls/tls_sw.c:957
sock_sendmsg_nosec net/socket.c:705 [inline]
sock_sendmsg net/socket.c:725 [inline]
__sys_sendto+0x42e/0x5b0 net/socket.c:2040
__do_sys_sendto net/socket.c:2052 [inline]
__se_sys_sendto net/socket.c:2048 [inline]
__x64_sys_sendto+0xda/0xf0 net/socket.c:2048
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x2b/0x70 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x44/0xae
RIP: 0033:0x7fd7940890e9
RSP: 002b:00007fd79523f168 EFLAGS: 00000246 ORIG_RAX: 000000000000002c
RAX: ffffffffffffffda RBX: 00007fd79419c030 RCX: 00007fd7940890e9
RDX: feffffff00000000 RSI: 0000000000000000 RDI: 0000000000000003
RBP: 00007fd7940e308d R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 00007fffdf66ea5f R14: 00007fd79523f300 R15: 0000000000022000
</TASK>
Showing all locks held in the system:
1 lock held by khungtaskd/28:
#0: ffffffff8cb1ae60 (rcu_read_lock){....}-{1:2}, at: rcu_lock_acquire+0x0/0x30
2 locks held by syslogd/2945:
#0: ffff8880b9b39c18 (&rq->__lock){-.-.}-{2:2}, at: raw_spin_rq_lock_nested+0x25/0x110 kernel/sched/core.c:554
#1: ffff8880b9b277c8 (&per_cpu_ptr(group->pcpu, cpu)->seq){-.-.}-{0:0}, at: psi_task_switch+0x567/0x820 kernel/sched/psi.c:889
2 locks held by getty/3270:
#0: ffff88814c440098 (&tty->ldisc_sem){++++}-{0:0}, at: tty_ldisc_ref_wait+0x21/0x70 drivers/tty/tty_ldisc.c:244
#1: ffffc90002b832e8 (&ldata->atomic_read_lock){+.+.}-{3:3}, at: n_tty_read+0x6ad/0x1c90 drivers/tty/n_tty.c:2075
3 locks held by kworker/0:3/3316:
#0: ffff888011464d38 ((wq_completion)events){+.+.}-{0:0}, at: process_one_work+0x796/0xd10 kernel/workqueue.c:2262
#1: ffffc90002f9fd00 ((work_completion)(&(&sw_ctx_tx->tx_work.work)->work)){+.+.}-{0:0}, at: process_one_work+0x7d0/0xd10 kernel/workqueue.c:2264
#2: ffff88807cb9a0d8 (&ctx->tx_lock){+.+.}-{3:3}, at: tx_work_handler+0x111/0x150 net/tls/tls_sw.c:2300
5 locks held by kworker/u4:6/3749:
1 lock held by syz-executor.3/5663:
#0: ffff88807cb9a0d8 (&ctx->tx_lock){+.+.}-{3:3}, at: tls_sw_sendmsg+0x297/0x1830 net/tls/tls_sw.c:957
=============================================
NMI backtrace for cpu 0
CPU: 0 PID: 28 Comm: khungtaskd Not tainted 5.18.0-rc3-syzkaller-00016-gb253435746d9 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:88 [inline]
dump_stack_lvl+0x1e3/0x2cb lib/dump_stack.c:106
nmi_cpu_backtrace+0x473/0x4a0 lib/nmi_backtrace.c:111
nmi_trigger_cpumask_backtrace+0x168/0x280 lib/nmi_backtrace.c:62
trigger_all_cpu_backtrace include/linux/nmi.h:146 [inline]
check_hung_uninterruptible_tasks kernel/hung_task.c:212 [inline]
watchdog+0xcf9/0xd40 kernel/hung_task.c:369
kthread+0x266/0x300 kernel/kthread.c:376
ret_from_fork+0x1f/0x30
</TASK>
Sending NMI from CPU 0 to CPUs 1:
NMI backtrace for cpu 1
CPU: 1 PID: 45 Comm: kworker/u4:2 Not tainted 5.18.0-rc3-syzkaller-00016-gb253435746d9 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Workqueue: phy7 ieee80211_iface_work
RIP: 0010:preempt_count_add+0x4e/0x180 kernel/sched/core.c:5537
Code: e3 00 00 00 83 3d b1 70 5c 0f 00 75 07 65 8b 05 90 39 a7 7e 65 01 1d 89 39 a7 7e 48 c7 c0 a0 a6 b7 90 48 c1 e8 03 42 8a 04 38 <84> c0 0f 85 db 00 00 00 83 3d 83 70 5c 0f 00 75 11 65 8b 05 62 39
RSP: 0018:ffffc9000115f2e0 EFLAGS: 00000a06
RAX: 1ffffffff216f404 RBX: 0000000000000001 RCX: ffffffff90b7a603
RDX: dffffc0000000000 RSI: ffffffff81d6a418 RDI: 0000000000000001
RBP: 1ffff9200022be78 R08: 0000000000000002 R09: ffffc9000115f4b0
R10: fffff5200022be84 R11: 1ffff9200022be82 R12: ffffc9000115f798
R13: ffffc9000115f3c0 R14: ffffffff81d6a418 R15: dffffc0000000000
FS: 0000000000000000(0000) GS:ffff8880b9b00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 000000c014c8aa20 CR3: 000000007e96d000 CR4: 00000000003506e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
<TASK>
unwind_next_frame+0xae/0x1dc0 arch/x86/kernel/unwind_orc.c:428
arch_stack_walk+0x112/0x140 arch/x86/kernel/stacktrace.c:25
stack_trace_save+0x12d/0x1f0 kernel/stacktrace.c:122
kasan_save_stack mm/kasan/common.c:38 [inline]
kasan_set_track+0x4c/0x70 mm/kasan/common.c:45
kasan_set_free_info+0x1f/0x40 mm/kasan/generic.c:370
____kasan_slab_free+0xd8/0x110 mm/kasan/common.c:366
kasan_slab_free include/linux/kasan.h:200 [inline]
slab_free_hook mm/slub.c:1728 [inline]
slab_free_freelist_hook+0x12e/0x1a0 mm/slub.c:1754
slab_free mm/slub.c:3510 [inline]
kfree+0xc6/0x210 mm/slub.c:4552
ieee80211_bss_info_update+0x96c/0xc30 net/mac80211/scan.c:232
ieee80211_rx_bss_info net/mac80211/ibss.c:1119 [inline]
ieee80211_rx_mgmt_probe_beacon net/mac80211/ibss.c:1610 [inline]
ieee80211_ibss_rx_queued_mgmt+0x1659/0x28c0 net/mac80211/ibss.c:1639
ieee80211_iface_process_skb net/mac80211/iface.c:1527 [inline]
ieee80211_iface_work+0x757/0xcd0 net/mac80211/iface.c:1581
process_one_work+0x81c/0xd10 kernel/workqueue.c:2289
worker_thread+0xb14/0x1330 kernel/workqueue.c:2436
kthread+0x266/0x300 kernel/kthread.c:376
ret_from_fork+0x1f/0x30
</TASK>
----------------
Code disassembly (best guess):
0: e3 00 jrcxz 0x2
2: 00 00 add %al,(%rax)
4: 83 3d b1 70 5c 0f 00 cmpl $0x0,0xf5c70b1(%rip) # 0xf5c70bc
b: 75 07 jne 0x14
d: 65 8b 05 90 39 a7 7e mov %gs:0x7ea73990(%rip),%eax # 0x7ea739a4
14: 65 01 1d 89 39 a7 7e add %ebx,%gs:0x7ea73989(%rip) # 0x7ea739a4
1b: 48 c7 c0 a0 a6 b7 90 mov $0xffffffff90b7a6a0,%rax
22: 48 c1 e8 03 shr $0x3,%rax
26: 42 8a 04 38 mov (%rax,%r15,1),%al
* 2a: 84 c0 test %al,%al <-- trapping instruction
2c: 0f 85 db 00 00 00 jne 0x10d
32: 83 3d 83 70 5c 0f 00 cmpl $0x0,0xf5c7083(%rip) # 0xf5c70bc
39: 75 11 jne 0x4c
3b: 65 gs
3c: 8b .byte 0x8b
3d: 05 .byte 0x5
3e: 62 .byte 0x62
3f: 39 .byte 0x39
---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
^ permalink raw reply
* Re: [PATCH v1 1/4] can: ctucanfd: remove PCI module debug parameters and core debug statements
From: Vincent Mailhol @ 2022-04-25 7:48 UTC (permalink / raw)
To: Pavel Pisa
Cc: linux-can, Marc Kleine-Budde, Oliver Hartkopp,
Wolfgang Grandegger, David Miller, netdev, linux-kernel,
Marin Jerabek, Ondrej Ille, Jiri Novak, Jaroslav Beran,
Petr Porazil, Pavel Machek, Carsten Emde, Drew Fustini,
Matej Vasilevski
In-Reply-To: <1fd684bcf5ddb0346aad234072f54e976a5210fb.1650816929.git.pisa@cmp.felk.cvut.cz>
Hi Pavel,
On Mon. 25 Apr. 2022 at 14:11, Pavel Pisa <pisa@cmp.felk.cvut.cz> wrote:
> This and remove of inline keyword from the local static functions
> should make happy all checks in actual versions of the both checkpatch.pl
> and patchwork tools.
The title and the description say two different things.
When looking at the code, it just seemed that you squashed
together two different patches: one to remove the inlines and one
to remove the debug. I guess you should split it again.
> Signed-off-by: Pavel Pisa <pisa@cmp.felk.cvut.cz>
> ---
> drivers/net/can/ctucanfd/ctucanfd_base.c | 33 +++---------------------
> drivers/net/can/ctucanfd/ctucanfd_pci.c | 22 +++++-----------
> 2 files changed, 9 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/net/can/ctucanfd/ctucanfd_base.c b/drivers/net/can/ctucanfd/ctucanfd_base.c
> index 7a4550f60abb..a1f6d37fca11 100644
> --- a/drivers/net/can/ctucanfd/ctucanfd_base.c
> +++ b/drivers/net/can/ctucanfd/ctucanfd_base.c
> @@ -133,13 +133,12 @@ static u32 ctucan_read32_be(struct ctucan_priv *priv,
> return ioread32be(priv->mem_base + reg);
> }
>
> -static inline void ctucan_write32(struct ctucan_priv *priv, enum ctu_can_fd_can_registers reg,
> - u32 val)
> +static void ctucan_write32(struct ctucan_priv *priv, enum ctu_can_fd_can_registers reg, u32 val)
> {
> priv->write_reg(priv, reg, val);
> }
>
> -static inline u32 ctucan_read32(struct ctucan_priv *priv, enum ctu_can_fd_can_registers reg)
> +static u32 ctucan_read32(struct ctucan_priv *priv, enum ctu_can_fd_can_registers reg)
> {
> return priv->read_reg(priv, reg);
> }
> @@ -179,8 +178,6 @@ static int ctucan_reset(struct net_device *ndev)
> struct ctucan_priv *priv = netdev_priv(ndev);
> int i = 100;
>
> - ctucan_netdev_dbg(ndev, "%s\n", __func__);
> -
> ctucan_write32(priv, CTUCANFD_MODE, REG_MODE_RST);
> clear_bit(CTUCANFD_FLAG_RX_FFW_BUFFERED, &priv->drv_flags);
>
> @@ -266,8 +263,6 @@ static int ctucan_set_bittiming(struct net_device *ndev)
> struct ctucan_priv *priv = netdev_priv(ndev);
> struct can_bittiming *bt = &priv->can.bittiming;
>
> - ctucan_netdev_dbg(ndev, "%s\n", __func__);
> -
> /* Note that bt may be modified here */
> return ctucan_set_btr(ndev, bt, true);
> }
> @@ -283,8 +278,6 @@ static int ctucan_set_data_bittiming(struct net_device *ndev)
> struct ctucan_priv *priv = netdev_priv(ndev);
> struct can_bittiming *dbt = &priv->can.data_bittiming;
>
> - ctucan_netdev_dbg(ndev, "%s\n", __func__);
> -
> /* Note that dbt may be modified here */
> return ctucan_set_btr(ndev, dbt, false);
> }
> @@ -302,8 +295,6 @@ static int ctucan_set_secondary_sample_point(struct net_device *ndev)
> int ssp_offset = 0;
> u32 ssp_cfg = 0; /* No SSP by default */
>
> - ctucan_netdev_dbg(ndev, "%s\n", __func__);
> -
> if (CTU_CAN_FD_ENABLED(priv)) {
> netdev_err(ndev, "BUG! Cannot set SSP - CAN is enabled\n");
> return -EPERM;
> @@ -390,8 +381,6 @@ static int ctucan_chip_start(struct net_device *ndev)
> int err;
> struct can_ctrlmode mode;
>
> - ctucan_netdev_dbg(ndev, "%s\n", __func__);
> -
> priv->txb_prio = 0x01234567;
> priv->txb_head = 0;
> priv->txb_tail = 0;
> @@ -457,8 +446,6 @@ static int ctucan_do_set_mode(struct net_device *ndev, enum can_mode mode)
> {
> int ret;
>
> - ctucan_netdev_dbg(ndev, "%s\n", __func__);
> -
> switch (mode) {
> case CAN_MODE_START:
> ret = ctucan_reset(ndev);
> @@ -486,7 +473,7 @@ static int ctucan_do_set_mode(struct net_device *ndev, enum can_mode mode)
> *
> * Return: Status of TXT buffer
> */
> -static inline enum ctucan_txtb_status ctucan_get_tx_status(struct ctucan_priv *priv, u8 buf)
> +static enum ctucan_txtb_status ctucan_get_tx_status(struct ctucan_priv *priv, u8 buf)
> {
> u32 tx_status = ctucan_read32(priv, CTUCANFD_TX_STATUS);
> enum ctucan_txtb_status status = (tx_status >> (buf * 4)) & 0x7;
> @@ -1123,8 +1110,6 @@ static irqreturn_t ctucan_interrupt(int irq, void *dev_id)
> u32 imask;
> int irq_loops;
>
> - ctucan_netdev_dbg(ndev, "%s\n", __func__);
> -
> for (irq_loops = 0; irq_loops < 10000; irq_loops++) {
> /* Get the interrupt status */
> isr = ctucan_read32(priv, CTUCANFD_INT_STAT);
> @@ -1198,8 +1183,6 @@ static void ctucan_chip_stop(struct net_device *ndev)
> u32 mask = 0xffffffff;
> u32 mode;
>
> - ctucan_netdev_dbg(ndev, "%s\n", __func__);
> -
> /* Disable interrupts and disable CAN */
> ctucan_write32(priv, CTUCANFD_INT_ENA_CLR, mask);
> ctucan_write32(priv, CTUCANFD_INT_MASK_SET, mask);
> @@ -1222,8 +1205,6 @@ static int ctucan_open(struct net_device *ndev)
> struct ctucan_priv *priv = netdev_priv(ndev);
> int ret;
>
> - ctucan_netdev_dbg(ndev, "%s\n", __func__);
> -
> ret = pm_runtime_get_sync(priv->dev);
> if (ret < 0) {
> netdev_err(ndev, "%s: pm_runtime_get failed(%d)\n",
> @@ -1283,8 +1264,6 @@ static int ctucan_close(struct net_device *ndev)
> {
> struct ctucan_priv *priv = netdev_priv(ndev);
>
> - ctucan_netdev_dbg(ndev, "%s\n", __func__);
> -
> netif_stop_queue(ndev);
> napi_disable(&priv->napi);
> ctucan_chip_stop(ndev);
> @@ -1310,8 +1289,6 @@ static int ctucan_get_berr_counter(const struct net_device *ndev, struct can_ber
> struct ctucan_priv *priv = netdev_priv(ndev);
> int ret;
>
> - ctucan_netdev_dbg(ndev, "%s\n", __func__);
> -
> ret = pm_runtime_get_sync(priv->dev);
> if (ret < 0) {
> netdev_err(ndev, "%s: pm_runtime_get failed(%d)\n", __func__, ret);
> @@ -1337,8 +1314,6 @@ int ctucan_suspend(struct device *dev)
> struct net_device *ndev = dev_get_drvdata(dev);
> struct ctucan_priv *priv = netdev_priv(ndev);
>
> - ctucan_netdev_dbg(ndev, "%s\n", __func__);
> -
> if (netif_running(ndev)) {
> netif_stop_queue(ndev);
> netif_device_detach(ndev);
> @@ -1355,8 +1330,6 @@ int ctucan_resume(struct device *dev)
> struct net_device *ndev = dev_get_drvdata(dev);
> struct ctucan_priv *priv = netdev_priv(ndev);
>
> - ctucan_netdev_dbg(ndev, "%s\n", __func__);
> -
> priv->can.state = CAN_STATE_ERROR_ACTIVE;
>
> if (netif_running(ndev)) {
> diff --git a/drivers/net/can/ctucanfd/ctucanfd_pci.c b/drivers/net/can/ctucanfd/ctucanfd_pci.c
> index c37a42480533..8f2956a8ae43 100644
> --- a/drivers/net/can/ctucanfd/ctucanfd_pci.c
> +++ b/drivers/net/can/ctucanfd/ctucanfd_pci.c
> @@ -45,14 +45,6 @@
> #define CTUCAN_WITHOUT_CTUCAN_ID 0
> #define CTUCAN_WITH_CTUCAN_ID 1
>
> -static bool use_msi = true;
> -module_param(use_msi, bool, 0444);
> -MODULE_PARM_DESC(use_msi, "PCIe implementation use MSI interrupts. Default: 1 (yes)");
> -
> -static bool pci_use_second = true;
> -module_param(pci_use_second, bool, 0444);
> -MODULE_PARM_DESC(pci_use_second, "Use the second CAN core on PCIe card. Default: 1 (yes)");
> -
> struct ctucan_pci_board_data {
> void __iomem *bar0_base;
> void __iomem *cra_base;
> @@ -117,13 +109,11 @@ static int ctucan_pci_probe(struct pci_dev *pdev,
> goto err_disable_device;
> }
>
> - if (use_msi) {
> - ret = pci_enable_msi(pdev);
> - if (!ret) {
> - dev_info(dev, "MSI enabled\n");
> - pci_set_master(pdev);
> - msi_ok = 1;
> - }
> + ret = pci_enable_msi(pdev);
> + if (!ret) {
> + dev_info(dev, "MSI enabled\n");
> + pci_set_master(pdev);
> + msi_ok = 1;
> }
>
> dev_info(dev, "ctucan BAR0 0x%08llx 0x%08llx\n",
> @@ -184,7 +174,7 @@ static int ctucan_pci_probe(struct pci_dev *pdev,
>
> core_i++;
>
> - while (pci_use_second && (core_i < num_cores)) {
> + while (core_i < num_cores) {
> addr += 0x4000;
> ret = ctucan_probe_common(dev, addr, irq, ntxbufs, 100000000,
> 0, ctucan_pci_set_drvdata);
> --
> 2.20.1
>
>
^ permalink raw reply
* Re: [RFC net-next] net: tc: flow indirect framework issue
From: Mattias Forsblad @ 2022-04-25 7:52 UTC (permalink / raw)
To: Jakub Kicinski, Pablo Neira Ayuso
Cc: Vladimir Oltean, Baowen Zheng, netdev@vger.kernel.org,
roid@nvidia.com, vladbu@nvidia.com, Eli Cohen, Jiri Pirko,
Tobias Waldekranz
In-Reply-To: <20220414105701.54c3fba4@kernel.org>
On 2022-04-14 10:57, Jakub Kicinski wrote:
>> I think some people believe doing things fully transparent is good, at
>> the cost of adding more kernel complexity and hiding details that are
>> relevant to the user (such as if hardware offload is enabled for
>> vxlan0 and what is the real device that is actually being used for the
>> vxlan0 to be offloaded).
>>
>> So, there are no flags when setting up the vxlan0 device for the user
>> to say: "I would like to hardware offload vxlan0", and going slightly
>> further there is not "please attach this vxlan0 device to eth0 for
>> hardware offload". Any real device could be potentially used to
>> offload vxlan0, the user does not know which one is actually used.
>>
>> Exposing this information is a bit more work on top of the user, but:
>>
>> 1) it will be transparent: the control plane shows that the vxlan0 is
>> hardware offloaded. Then if eth0 is gone, vxlan0 tc ingress can be
>> removed too, because it depends on eth0.
>>
>> 2) The control plane validates if hardware offload for vxlan0. If this
>> is not possible, display an error to the user: "sorry, I cannot
>> offload vxlan0 on eth0 for reason X".
>>
>> Since this is not exposed to the control plane, the existing
>> infrastructure follows a snooping scheme, but tracking devices that
>> might be able to hardware offload.
>>
>> There is no obvious way to relate vxlan0 with the real device
>> (eth0) that is actually performing the hardware offloading.
>
> Let's not over-complicate things, Mattias just needs replay to work.
> 90% sure it worked when we did the work back in the day with John H,
> before the nft rewrite etc.
To me the first thing to determine is how flow_indr_dev_register should work?
With only a superficial knowledge of tc I'd seem to me that if we
have a function called tcf_action_reoffload_cb and tc has all the information
about current blocks/filters/rules it should really reoffload those. The other way
would mean bookkeeping the same information at multiple places. It also
means restrictions on which sequence one should setup a network topology.
Would we like it that way?
^ permalink raw reply
* [PATCH bpf-next] bpftoo: Support user defined vmlinux path
From: Jianlin Lv @ 2022-04-25 7:57 UTC (permalink / raw)
To: bpf
Cc: ast, daniel, andrii, kafai, quentin, jean-philippe, mauricio,
ytcoode, linux-kernel, netdev, jianlv, Jianlin Lv
From: Jianlin Lv <iecedge@gmail.com>
Add EXTERNAL_PATH variable that define unconventional vmlinux path
Signed-off-by: Jianlin Lv <iecedge@gmail.com>
---
When building Ubuntu-5.15.0 kernel, '../../../vmlinux' cannot locate
compiled vmlinux image. Incorrect vmlinux generated vmlinux.h missing some
structure definitions that broken compiling pipe.
---
tools/bpf/bpftool/Makefile | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
index c6d2c77d0252..fefa3b763eb7 100644
--- a/tools/bpf/bpftool/Makefile
+++ b/tools/bpf/bpftool/Makefile
@@ -160,6 +160,7 @@ $(OBJS): $(LIBBPF) $(LIBBPF_INTERNAL_HDRS)
VMLINUX_BTF_PATHS ?= $(if $(O),$(O)/vmlinux) \
$(if $(KBUILD_OUTPUT),$(KBUILD_OUTPUT)/vmlinux) \
../../../vmlinux \
+ $(if $(EXTERNAL_PATH),$(EXTERNAL_PATH)/vmlinux) \
/sys/kernel/btf/vmlinux \
/boot/vmlinux-$(shell uname -r)
VMLINUX_BTF ?= $(abspath $(firstword $(wildcard $(VMLINUX_BTF_PATHS))))
--
2.25.1
^ permalink raw reply related
* Re: [PATCH] drivers: nfc: nfcmrvl: reorder destructive operations in nfcmrvl_nci_unregister_dev to avoid bugs
From: Krzysztof Kozlowski @ 2022-04-25 7:59 UTC (permalink / raw)
To: Duoming Zhou, linux-kernel
Cc: netdev, broonie, akpm, alexander.deucher, gregkh, davem, linma
In-Reply-To: <20220425031002.56254-1-duoming@zju.edu.cn>
On 25/04/2022 05:10, Duoming Zhou wrote:
> There are destructive operations such as nfcmrvl_fw_dnld_abort and
> gpio_free in nfcmrvl_nci_unregister_dev. The resources such as firmware,
> gpio and so on could be destructed while the upper layer functions such as
> nfcmrvl_fw_dnld_start and nfcmrvl_nci_recv_frame is executing, which leads
> to double-free, use-after-free and null-ptr-deref bugs.
You need to correct the subject prefix because it does not describe the
subsystem (git log oneline --).
>
> There are three situations that could lead to double-free bugs.
>
> The first situation is shown below:
>
> (Thread 1) | (Thread 2)
> nfcmrvl_fw_dnld_start |
> ... | nfcmrvl_nci_unregister_dev
> release_firmware() | nfcmrvl_fw_dnld_abort
> kfree(fw) //(1) | fw_dnld_over
> | release_firmware
> ... | kfree(fw) //(2)
> | ...
>
> The second situation is shown below:
>
> (Thread 1) | (Thread 2)
> nfcmrvl_fw_dnld_start |
> ... |
> mod_timer |
> (wait a time) |
> fw_dnld_timeout | nfcmrvl_nci_unregister_dev
> fw_dnld_over | nfcmrvl_fw_dnld_abort
> release_firmware | fw_dnld_over
> kfree(fw) //(1) | release_firmware
> ... | kfree(fw) //(2)
>
> The third situation is shown below:
>
> (Thread 1) | (Thread 2)
> nfcmrvl_nci_recv_frame |
> if(..->fw_download_in_progress)|
> nfcmrvl_fw_dnld_recv_frame |
> queue_work |
> |
> fw_dnld_rx_work | nfcmrvl_nci_unregister_dev
> fw_dnld_over | nfcmrvl_fw_dnld_abort
> release_firmware | fw_dnld_over
> kfree(fw) //(1) | release_firmware
> | kfree(fw) //(2)
>
> The firmware struct is deallocated in position (1) and deallocated
> in position (2) again.
>
> The crash trace triggered by POC is like below:
>
> [ 122.640457] BUG: KASAN: double-free or invalid-free in fw_dnld_over+0x28/0xf0
> [ 122.640457] Call Trace:
> [ 122.640457] <TASK>
> [ 122.640457] kfree+0xb0/0x330
> [ 122.640457] fw_dnld_over+0x28/0xf0
> [ 122.640457] nfcmrvl_nci_unregister_dev+0x61/0x70
> [ 122.640457] nci_uart_tty_close+0x87/0xd0
> [ 122.640457] tty_ldisc_kill+0x3e/0x80
> [ 122.640457] tty_ldisc_hangup+0x1b2/0x2c0
> [ 122.640457] __tty_hangup.part.0+0x316/0x520
> [ 122.640457] tty_release+0x200/0x670
> [ 122.640457] __fput+0x110/0x410
> [ 122.640457] task_work_run+0x86/0xd0
> [ 122.640457] exit_to_user_mode_prepare+0x1aa/0x1b0
> [ 122.640457] syscall_exit_to_user_mode+0x19/0x50
> [ 122.640457] do_syscall_64+0x48/0x90
> [ 122.640457] entry_SYSCALL_64_after_hwframe+0x44/0xae
> [ 122.640457] RIP: 0033:0x7f68433f6beb
>
> What's more, there are also use-after-free and null-ptr-deref bugs
> in nfcmrvl_fw_dnld_start. If we deallocate firmware struct, gpio or
> set null to the members of priv->fw_dnld in nfcmrvl_nci_unregister_dev,
> then, we dereference firmware, gpio or the members of priv->fw_dnld in
> nfcmrvl_fw_dnld_start, the UAF or NPD bugs will happen.
>
> This patch reorders destructive operations after nci_unregister_device
> to avoid the double-free, UAF and NPD bugs, as nci_unregister_device
> is well synchronized and won't return if there is a running routine.
> This was mentioned in commit 3e3b5dfcd16a ("NFC: reorder the logic in
> nfc_{un,}register_device").
>
> Fixes: 3194c6870158 ("NFC: nfcmrvl: add firmware download support")
> Signed-off-by: Duoming Zhou <duoming@zju.edu.cn>
> Reviewed-by: Lin Ma <linma@zju.edu.cn>
It's the first submission, how this review appeared?
On the other hand, you already sent something similar, so is it a v2? I
am sorry but this is very confusing. Looks like
https://lore.kernel.org/all/20220425005718.33639-1-duoming@zju.edu.cn/
but there is no changelog and commit description is different.
Please read:
https://elixir.bootlin.com/linux/v5.18-rc4/source/Documentation/process/submitting-patches.rst
> ---
> drivers/nfc/nfcmrvl/main.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v1 1/4] can: ctucanfd: remove PCI module debug parameters and core debug statements
From: Pavel Pisa @ 2022-04-25 8:10 UTC (permalink / raw)
To: Vincent Mailhol, linux-can, Marc Kleine-Budde, Oliver Hartkopp
Cc: Wolfgang Grandegger, David Miller, netdev, linux-kernel,
Marin Jerabek, Ondrej Ille, Jiri Novak, Jaroslav Beran,
Petr Porazil, Pavel Machek, Carsten Emde, Drew Fustini,
Matej Vasilevski
In-Reply-To: <CAMZ6RqJ1ROr-pLsJqKE=dK=cVD+-KGxSj1wPEZY-AXH9_d4xyQ@mail.gmail.com>
Hello Vincent,
On Monday 25 of April 2022 09:48:51 Vincent Mailhol wrote:
> On Mon. 25 Apr. 2022 at 14:11, Pavel Pisa <pisa@cmp.felk.cvut.cz> wrote:
> > This and remove of inline keyword from the local static functions
> > should make happy all checks in actual versions of the both checkpatch.pl
> > and patchwork tools.
>
> The title and the description say two different things.
>
> When looking at the code, it just seemed that you squashed
> together two different patches: one to remove the inlines and one
> to remove the debug. I guess you should split it again.
if you or somebody else confirms that the three lines change
worth separate patch I regenerate the series.
The changes are not based on third party patches but only
on indications reported by static analysis tools.
Remove of inline in the local static functions probably
does not even change code generation by current compiler
generation. Removed debug outputs are under local ifdef
disabled by default, so only real change is step down from
option to use module parameter to check for possible
broken MSI causing the problems on PCIe CTU CAN FD integration.
So I thought that single relatively small cleanup patch is
less load to maintainers.
But I have no strong preference there and will do as confirmed.
By the way, what is preference for CC, should the series
be sent to linux-kernel and netdev or it is preferred for these
local changes to send it only to linux-can to not load others?
Same for CC to David Miller.
Best wishes,
Pavel
--
Pavel Pisa
phone: +420 603531357
e-mail: pisa@cmp.felk.cvut.cz
Department of Control Engineering FEE CVUT
Karlovo namesti 13, 121 35, Prague 2
university: http://control.fel.cvut.cz/
personal: http://cmp.felk.cvut.cz/~pisa
projects: https://www.openhub.net/accounts/ppisa
CAN related:http://canbus.pages.fel.cvut.cz/
Open Technologies Research Education and Exchange Services
https://gitlab.fel.cvut.cz/otrees/org/-/wikis/home
^ permalink raw reply
* Re: [PATCH v1 1/4] can: ctucanfd: remove PCI module debug parameters and core debug statements
From: Vincent Mailhol @ 2022-04-25 8:34 UTC (permalink / raw)
To: Pavel Pisa
Cc: linux-can, Marc Kleine-Budde, Oliver Hartkopp,
Wolfgang Grandegger, David Miller, netdev, linux-kernel,
Marin Jerabek, Ondrej Ille, Jiri Novak, Jaroslav Beran,
Petr Porazil, Pavel Machek, Carsten Emde, Drew Fustini,
Matej Vasilevski
In-Reply-To: <202204251010.39032.pisa@cmp.felk.cvut.cz>
On Mon. 25 Apr 2022 at 17:10, Pavel Pisa <pisa@cmp.felk.cvut.cz> wrote:
> Hello Vincent,
>
> On Monday 25 of April 2022 09:48:51 Vincent Mailhol wrote:
> > On Mon. 25 Apr. 2022 at 14:11, Pavel Pisa <pisa@cmp.felk.cvut.cz> wrote:
> > > This and remove of inline keyword from the local static functions
> > > should make happy all checks in actual versions of the both checkpatch.pl
> > > and patchwork tools.
> >
> > The title and the description say two different things.
> >
> > When looking at the code, it just seemed that you squashed
> > together two different patches: one to remove the inlines and one
> > to remove the debug. I guess you should split it again.
>
> if you or somebody else confirms that the three lines change
> worth separate patch I regenerate the series.
I was just troubled that the title was saying "remove debug" and
that the body was saying "remove inline". I genuinely thought
that you inadvertently squashed two different patches
together.
I just expect the body of the patch to give extended explanations
of what is in the title, not to introduce something else, and
this regardless of the number of lines being changed.
> The changes are not based on third party patches but only
> on indications reported by static analysis tools.
> Remove of inline in the local static functions probably
> does not even change code generation by current compiler
> generation. Removed debug outputs are under local ifdef
> disabled by default, so only real change is step down from
> option to use module parameter to check for possible
> broken MSI causing the problems on PCIe CTU CAN FD integration.
> So I thought that single relatively small cleanup patch is
> less load to maintainers.
>
> But I have no strong preference there and will do as confirmed.
>
> By the way, what is preference for CC, should the series
> be sent to linux-kernel and netdev or it is preferred for these
> local changes to send it only to linux-can to not load others?
> Same for CC to David Miller.
I used to include them in the past because of
get_maitainer.pl. But Oliver pointed out that it is not
necessary. Now, I just sent it to linux-can and Marc (and maybe
some driver maintainers when relevant).
> Best wishes,
>
> Pavel
> --
> Pavel Pisa
> phone: +420 603531357
> e-mail: pisa@cmp.felk.cvut.cz
> Department of Control Engineering FEE CVUT
> Karlovo namesti 13, 121 35, Prague 2
> university: http://control.fel.cvut.cz/
> personal: http://cmp.felk.cvut.cz/~pisa
> projects: https://www.openhub.net/accounts/ppisa
> CAN related:http://canbus.pages.fel.cvut.cz/
> Open Technologies Research Education and Exchange Services
> https://gitlab.fel.cvut.cz/otrees/org/-/wikis/home
>
^ permalink raw reply
* Re: ctcm: rename READ/WRITE defines to avoid redefinitions
From: Alexandra Winter @ 2022-04-25 8:38 UTC (permalink / raw)
To: Colin King (gmail), Wenjia Zhang, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev
Cc: Christian Borntraeger, linux-s390, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <ae612043-0252-e8c3-0773-912f116421c1@gmail.com>
On 24.04.22 20:58, Colin King (gmail) wrote:
> Hi,
>
> static analysis with cppcheck detected a potential null pointer deference with the following commit:
>
> commit 3c09e2647b5e1f1f9fd383971468823c2505e1b0
> Author: Ursula Braun <ursula.braun@de.ibm.com>
> Date: Thu Aug 12 01:58:28 2010 +0000
>
> ctcm: rename READ/WRITE defines to avoid redefinitions
>
>
> The analysis is as follows:
>
> drivers/s390/net/ctcm_sysfs.c:43:8: note: Assuming that condition 'priv' is not redundant
> if (!(priv && priv->channel[CTCM_READ] && ndev)) {
> ^
> drivers/s390/net/ctcm_sysfs.c:42:9: note: Null pointer dereference
> ndev = priv->channel[CTCM_READ]->netdev;
>
> The code in question is as follows:
>
> ndev = priv->channel[CTCM_READ]->netdev;
>
> ^^ priv may be null, as per check below but it is being dereferenced when assigning ndev
>
> if (!(priv && priv->channel[CTCM_READ] && ndev)) {
> CTCM_DBF_TEXT(SETUP, CTC_DBF_ERROR, "bfnondev");
> return -ENODEV;
> }
>
> Colin
Thank you very much for reporting this, we will provide a patch.
Do you have any special requests for the Reported-by flag? Or is
Reported-by: Colin King (gmail) <colin.i.king@gmail.com>
fine with you?
Kind regards
Alexandra
^ 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