* Re: [PATCH 4/5] net: phy: marvell: Add LED accessors for Marvell 88E1510
From: Kai-Heng Feng @ 2022-04-21 3:11 UTC (permalink / raw)
To: Andrew Lunn
Cc: hkallweit1, linux, peppe.cavallaro, alexandre.torgue, joabreu,
davem, kuba, pabeni, netdev, linux-kernel
In-Reply-To: <YmAgq1pm37Glw2v+@lunn.ch>
On Wed, Apr 20, 2022 at 11:03 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Wed, Apr 20, 2022 at 08:40:51PM +0800, Kai-Heng Feng wrote:
> > Implement get_led_config() and set_led_config() callbacks so phy core
> > can use firmware LED as platform requested.
> >
> > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> > ---
> > drivers/net/phy/marvell.c | 26 ++++++++++++++++++++++++++
> > 1 file changed, 26 insertions(+)
> >
> > diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
> > index 2702faf7b0f60..c5f13e09b0692 100644
> > --- a/drivers/net/phy/marvell.c
> > +++ b/drivers/net/phy/marvell.c
> > @@ -750,6 +750,30 @@ static int m88e1510_config_aneg(struct phy_device *phydev)
> > return err;
> > }
> >
> > +static int marvell_get_led_config(struct phy_device *phydev)
> > +{
> > + int led;
> > +
> > + led = phy_read_paged(phydev, MII_MARVELL_LED_PAGE, MII_PHY_LED_CTRL);
> > + if (led < 0) {
> > + phydev_warn(phydev, "Fail to get marvell phy LED.\n");
> > + led = 0;
> > + }
>
> I've said this multiple times, there are three LED registers, The
> Function Control register, the Priority Control register and the Timer
> control register. It is the combination of all three that defines how
> the LEDs work. You need to save all of them.
OK, will do.
>
> And you need to make your API generic enough that the PHY driver can
> save anywhere from 1 bit to 42 bytes of configuration.
OK, I guess I'll let the implementation stays within driver callback,
so each driver can decide how many bits need to be saved.
>
> I don't know ACPI, but i'm pretty sure this is not the ACPI way of
> doing this. I think you should be defining an ACPI method, which
> phylib can call after initialising the hardware to allow the firmware
> to configure the LED.
This is not feasible.
If BIOS can define a method and restore the LED by itself, it can put
the method inside its S3 method and I don't have to work on this at
the first place.
Kai-Heng
>
> Andrew
^ permalink raw reply
* Re: [PATCH 2/5] net: mdio: Add "use-firmware-led" firmware property
From: Kai-Heng Feng @ 2022-04-21 3:15 UTC (permalink / raw)
To: Andrew Lunn
Cc: hkallweit1, linux, peppe.cavallaro, alexandre.torgue, joabreu,
davem, kuba, pabeni, netdev, linux-kernel
In-Reply-To: <YmAEDbJJU1hLNSY1@lunn.ch>
On Wed, Apr 20, 2022 at 9:01 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Wed, Apr 20, 2022 at 08:40:49PM +0800, Kai-Heng Feng wrote:
> > Some system may prefer preset PHY LED setting instead of the one
> > hardcoded in the PHY driver, so adding a new firmware
> > property, "use-firmware-led", to cope with that.
> >
> > On ACPI based system the ASL looks like this:
> >
> > Scope (_SB.PC00.OTN0)
> > {
> > Device (PHY0)
> > {
> > Name (_ADR, One) // _ADR: Address
> > Name (_DSD, Package (0x02) // _DSD: Device-Specific Data
> > {
> > ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301") /* Device Properties for _DSD */,
> > Package (0x01)
> > {
> > Package (0x02)
> > {
> > "use-firmware-led",
> > One
> > }
> > }
> > })
> > }
> >
> > Name (_DSD, Package (0x02) // _DSD: Device-Specific Data
> > {
> > ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301") /* Device Properties for _DSD */,
> > Package (0x01)
> > {
> > Package (0x02)
> > {
> > "phy-handle",
> > PHY0
> > }
> > }
> > })
> > }
> >
> > Basically use the "phy-handle" reference to read the "use-firmware-led"
> > boolean.
>
> Please update Documentation/firmware-guide/acpi/dsd/phy.rst
>
> Please also Cc: the ACPI list. I have no idea what the naming
> convention is for ACPI properties.
>
> >
> > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> > ---
> > drivers/net/mdio/fwnode_mdio.c | 4 ++++
> > include/linux/phy.h | 1 +
> > 2 files changed, 5 insertions(+)
> >
> > diff --git a/drivers/net/mdio/fwnode_mdio.c b/drivers/net/mdio/fwnode_mdio.c
> > index 1c1584fca6327..bfca67b42164b 100644
> > --- a/drivers/net/mdio/fwnode_mdio.c
> > +++ b/drivers/net/mdio/fwnode_mdio.c
> > @@ -144,6 +144,10 @@ int fwnode_mdiobus_register_phy(struct mii_bus *bus,
> > */
> > if (mii_ts)
> > phy->mii_ts = mii_ts;
> > +
> > + phy->use_firmware_led =
> > + fwnode_property_read_bool(child, "use-firmware-led");
> > +
>
> This is an ACPI only property. It is not valid in DT. It does not
> fulfil the DT naming requirement etc. So please move this up inside
> the if (is_acpi_node(child)) clause.
>
> > diff --git a/include/linux/phy.h b/include/linux/phy.h
> > index 36ca2b5c22533..53e693b3430ec 100644
> > --- a/include/linux/phy.h
> > +++ b/include/linux/phy.h
> > @@ -656,6 +656,7 @@ struct phy_device {
> > /* Energy efficient ethernet modes which should be prohibited */
> > u32 eee_broken_modes;
> >
> > + bool use_firmware_led;
>
> There is probably a two byte hole after mdix_ctrl. So please consider
> adding it there. Also, don't forget to update the documentation.
OK, will do once other concerns are ironed out.
Kai-Heng
>
> Andrew
^ permalink raw reply
* [PATCH -next] libbpf: Remove redundant non-null checks on obj_elf
From: Gaosheng Cui @ 2022-04-21 3:18 UTC (permalink / raw)
To: cuigaosheng1, ast, daniel, andrii, kafai, songliubraving, yhs,
john.fastabend, kpsingh
Cc: netdev, bpf, linux-kernel, gongruiqi1, wangweiyang2
Obj_elf is already non-null checked at the function entry, so remove
redundant non-null checks on obj_elf.
Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com>
---
tools/lib/bpf/libbpf.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index bf4f7ac54ebf..b53e51884f9e 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -1222,10 +1222,8 @@ static void bpf_object__elf_finish(struct bpf_object *obj)
if (!obj->efile.elf)
return;
- if (obj->efile.elf) {
- elf_end(obj->efile.elf);
- obj->efile.elf = NULL;
- }
+ elf_end(obj->efile.elf);
+ obj->efile.elf = NULL;
obj->efile.symbols = NULL;
obj->efile.st_ops_data = NULL;
--
2.25.1
^ permalink raw reply related
* Re: [PATCH net v2] ice: Fix race during aux device (un)plugging
From: Leon Romanovsky @ 2022-04-21 5:12 UTC (permalink / raw)
To: Ivan Vecera
Cc: netdev, poros, mschmidt, Jesse Brandeburg, Tony Nguyen,
David S. Miller, Jakub Kicinski, Paolo Abeni, Dave Ertman,
Shiraz Saleem, moderated list:INTEL ETHERNET DRIVERS, open list
In-Reply-To: <20220420150300.1062689-1-ivecera@redhat.com>
On Wed, Apr 20, 2022 at 05:02:59PM +0200, Ivan Vecera wrote:
> Function ice_plug_aux_dev() assigns pf->adev field too early prior
> aux device initialization and on other side ice_unplug_aux_dev()
> starts aux device deinit and at the end assigns NULL to pf->adev.
> This is wrong because pf->adev should always be non-NULL only when
> aux device is fully initialized and ready. This wrong order causes
> a crash when ice_send_event_to_aux() call occurs because that function
> depends on non-NULL value of pf->adev and does not assume that
> aux device is half-initialized or half-destroyed.
> After order correction the race window is tiny but it is still there,
> as Leon mentioned and manipulation with pf->adev needs to be protected
> by mutex.
>
> Fix (un-)plugging functions so pf->adev field is set after aux device
> init and prior aux device destroy and protect pf->adev assignment by
> new mutex. This mutex is also held during ice_send_event_to_aux()
> call to ensure that aux device is valid during that call. Device
> lock used ice_send_event_to_aux() to avoid its concurrent run can
> be removed as this is secured by that mutex.
>
> Reproducer:
> cycle=1
> while :;do
> echo "#### Cycle: $cycle"
>
> ip link set ens7f0 mtu 9000
> ip link add bond0 type bond mode 1 miimon 100
> ip link set bond0 up
> ifenslave bond0 ens7f0
> ip link set bond0 mtu 9000
> ethtool -L ens7f0 combined 1
> ip link del bond0
> ip link set ens7f0 mtu 1500
> sleep 1
>
> let cycle++
> done
>
> In short when the device is added/removed to/from bond the aux device
> is unplugged/plugged. When MTU of the device is changed an event is
> sent to aux device asynchronously. This can race with (un)plugging
> operation and because pf->adev is set too early (plug) or too late
> (unplug) the function ice_send_event_to_aux() can touch uninitialized
> or destroyed fields. In the case of crash below pf->adev->dev.mutex.
>
> Crash:
> [ 53.372066] bond0: (slave ens7f0): making interface the new active one
> [ 53.378622] bond0: (slave ens7f0): Enslaving as an active interface with an u
> p link
> [ 53.386294] IPv6: ADDRCONF(NETDEV_CHANGE): bond0: link becomes ready
> [ 53.549104] bond0: (slave ens7f1): Enslaving as a backup interface with an up
> link
> [ 54.118906] ice 0000:ca:00.0 ens7f0: Number of in use tx queues changed inval
> idating tc mappings. Priority traffic classification disabled!
> [ 54.233374] ice 0000:ca:00.1 ens7f1: Number of in use tx queues changed inval
> idating tc mappings. Priority traffic classification disabled!
> [ 54.248204] bond0: (slave ens7f0): Releasing backup interface
> [ 54.253955] bond0: (slave ens7f1): making interface the new active one
> [ 54.274875] bond0: (slave ens7f1): Releasing backup interface
> [ 54.289153] bond0 (unregistering): Released all slaves
> [ 55.383179] MII link monitoring set to 100 ms
> [ 55.398696] bond0: (slave ens7f0): making interface the new active one
> [ 55.405241] BUG: kernel NULL pointer dereference, address: 0000000000000080
> [ 55.405289] bond0: (slave ens7f0): Enslaving as an active interface with an u
> p link
> [ 55.412198] #PF: supervisor write access in kernel mode
> [ 55.412200] #PF: error_code(0x0002) - not-present page
> [ 55.412201] PGD 25d2ad067 P4D 0
> [ 55.412204] Oops: 0002 [#1] PREEMPT SMP NOPTI
> [ 55.412207] CPU: 0 PID: 403 Comm: kworker/0:2 Kdump: loaded Tainted: G S
> 5.17.0-13579-g57f2d6540f03 #1
> [ 55.429094] bond0: (slave ens7f1): Enslaving as a backup interface with an up
> link
> [ 55.430224] Hardware name: Dell Inc. PowerEdge R750/06V45N, BIOS 1.4.4 10/07/
> 2021
> [ 55.430226] Workqueue: ice ice_service_task [ice]
> [ 55.468169] RIP: 0010:mutex_unlock+0x10/0x20
> [ 55.472439] Code: 0f b1 13 74 96 eb e0 4c 89 ee eb d8 e8 79 54 ff ff 66 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 65 48 8b 04 25 40 ef 01 00 31 d2 <f0> 48 0f b1 17 75 01 c3 e9 e3 fe ff ff 0f 1f 00 0f 1f 44 00 00 48
> [ 55.491186] RSP: 0018:ff4454230d7d7e28 EFLAGS: 00010246
> [ 55.496413] RAX: ff1a79b208b08000 RBX: ff1a79b2182e8880 RCX: 0000000000000001
> [ 55.503545] RDX: 0000000000000000 RSI: ff4454230d7d7db0 RDI: 0000000000000080
> [ 55.510678] RBP: ff1a79d1c7e48b68 R08: ff4454230d7d7db0 R09: 0000000000000041
> [ 55.517812] R10: 00000000000000a5 R11: 00000000000006e6 R12: ff1a79d1c7e48bc0
> [ 55.524945] R13: 0000000000000000 R14: ff1a79d0ffc305c0 R15: 0000000000000000
> [ 55.532076] FS: 0000000000000000(0000) GS:ff1a79d0ffc00000(0000) knlGS:0000000000000000
> [ 55.540163] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 55.545908] CR2: 0000000000000080 CR3: 00000003487ae003 CR4: 0000000000771ef0
> [ 55.553041] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [ 55.560173] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> [ 55.567305] PKRU: 55555554
> [ 55.570018] Call Trace:
> [ 55.572474] <TASK>
> [ 55.574579] ice_service_task+0xaab/0xef0 [ice]
> [ 55.579130] process_one_work+0x1c5/0x390
> [ 55.583141] ? process_one_work+0x390/0x390
> [ 55.587326] worker_thread+0x30/0x360
> [ 55.590994] ? process_one_work+0x390/0x390
> [ 55.595180] kthread+0xe6/0x110
> [ 55.598325] ? kthread_complete_and_exit+0x20/0x20
> [ 55.603116] ret_from_fork+0x1f/0x30
> [ 55.606698] </TASK>
>
> Fixes: f9f5301e7e2d ("ice: Register auxiliary device to provide RDMA")
> Cc: Leon Romanovsky <leon@kernel.org>
> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
> ---
> drivers/net/ethernet/intel/ice/ice.h | 1 +
> drivers/net/ethernet/intel/ice/ice_idc.c | 33 ++++++++++++++---------
> drivers/net/ethernet/intel/ice/ice_main.c | 2 ++
> 3 files changed, 23 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h
> index 8ed3c9ab7ff7..a895e3a8e988 100644
> --- a/drivers/net/ethernet/intel/ice/ice.h
> +++ b/drivers/net/ethernet/intel/ice/ice.h
> @@ -540,6 +540,7 @@ struct ice_pf {
> struct mutex avail_q_mutex; /* protects access to avail_[rx|tx]qs */
> struct mutex sw_mutex; /* lock for protecting VSI alloc flow */
> struct mutex tc_mutex; /* lock to protect TC changes */
> + struct mutex adev_mutex; /* lock to protect aux device access */
> u32 msg_enable;
> struct ice_ptp ptp;
> struct tty_driver *ice_gnss_tty_driver;
> diff --git a/drivers/net/ethernet/intel/ice/ice_idc.c b/drivers/net/ethernet/intel/ice/ice_idc.c
> index 25a436d342c2..aef07accd63b 100644
> --- a/drivers/net/ethernet/intel/ice/ice_idc.c
> +++ b/drivers/net/ethernet/intel/ice/ice_idc.c
> @@ -10,13 +10,15 @@
> * ice_get_auxiliary_drv - retrieve iidc_auxiliary_drv struct
> * @pf: pointer to PF struct
> *
> - * This function has to be called with a device_lock on the
> - * pf->adev.dev to avoid race conditions.
> + * This function has to be called with pf->adev_mutex held
> + * to avoid race conditions.
> */
> static struct iidc_auxiliary_drv *ice_get_auxiliary_drv(struct ice_pf *pf)
> {
> struct auxiliary_device *adev;
>
> + BUG_ON(!mutex_is_locked(&pf->adev_mutex));
Please don't add BUG_ON() in driver code.
I think that you are looking for lockdep_assert_held(&pf->adev_mutex).
Thanks
^ permalink raw reply
* [PATCH] net: hns: Add missing fwnode_handle_put in hns_mac_init
From: Peng Wu @ 2022-04-21 5:53 UTC (permalink / raw)
To: yisen.zhuang, salil.mehta, huangguangbin2, shenyang39, lipeng321
Cc: netdev, linux-kernel, wupeng58, liwei391
In one of the error paths of the device_for_each_child_node() loop
in hns_mac_init, add missing call to fwnode_handle_put.
Signed-off-by: Peng Wu <wupeng58@huawei.com>
---
drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c
index 7edf8569514c..928d934cb21a 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c
@@ -1065,19 +1065,23 @@ int hns_mac_init(struct dsaf_device *dsaf_dev)
device_for_each_child_node(dsaf_dev->dev, child) {
ret = fwnode_property_read_u32(child, "reg", &port_id);
if (ret) {
+ fwnode_handle_put(child);
dev_err(dsaf_dev->dev,
"get reg fail, ret=%d!\n", ret);
return ret;
}
if (port_id >= max_port_num) {
+ fwnode_handle_put(child);
dev_err(dsaf_dev->dev,
"reg(%u) out of range!\n", port_id);
return -EINVAL;
}
mac_cb = devm_kzalloc(dsaf_dev->dev, sizeof(*mac_cb),
GFP_KERNEL);
- if (!mac_cb)
+ if (!mac_cb) {
+ fwnode_handle_put(child);
return -ENOMEM;
+ }
mac_cb->fw_port = child;
mac_cb->mac_id = (u8)port_id;
dsaf_dev->mac_cb[port_id] = mac_cb;
--
2.17.1
^ permalink raw reply related
* Re: [PATCH net v2] ice: Fix race during aux device (un)plugging
From: Ivan Vecera @ 2022-04-21 6:08 UTC (permalink / raw)
To: Leon Romanovsky
Cc: netdev, poros, mschmidt, Jesse Brandeburg, Tony Nguyen,
David S. Miller, Jakub Kicinski, Paolo Abeni, Dave Ertman,
Shiraz Saleem, moderated list:INTEL ETHERNET DRIVERS, open list
In-Reply-To: <YmDn2ptpRHasOQag@unreal>
On Thu, 21 Apr 2022 08:12:58 +0300
Leon Romanovsky <leon@kernel.org> wrote:
> > static struct iidc_auxiliary_drv *ice_get_auxiliary_drv(struct ice_pf *pf)
> > {
> > struct auxiliary_device *adev;
> >
> > + BUG_ON(!mutex_is_locked(&pf->adev_mutex));
>
> Please don't add BUG_ON() in driver code.
>
> I think that you are looking for lockdep_assert_held(&pf->adev_mutex).
Will fix.
I.
^ permalink raw reply
* [PATCH net v3] ice: Fix race during aux device (un)plugging
From: Ivan Vecera @ 2022-04-21 6:09 UTC (permalink / raw)
To: netdev
Cc: poros, mschmidt, Leon Romanovsky, Jesse Brandeburg, Tony Nguyen,
David S. Miller, Jakub Kicinski, Paolo Abeni, Dave Ertman,
Shiraz Saleem, moderated list:INTEL ETHERNET DRIVERS, open list
Function ice_plug_aux_dev() assigns pf->adev field too early prior
aux device initialization and on other side ice_unplug_aux_dev()
starts aux device deinit and at the end assigns NULL to pf->adev.
This is wrong because pf->adev should always be non-NULL only when
aux device is fully initialized and ready. This wrong order causes
a crash when ice_send_event_to_aux() call occurs because that function
depends on non-NULL value of pf->adev and does not assume that
aux device is half-initialized or half-destroyed.
After order correction the race window is tiny but it is still there,
as Leon mentioned and manipulation with pf->adev needs to be protected
by mutex.
Fix (un-)plugging functions so pf->adev field is set after aux device
init and prior aux device destroy and protect pf->adev assignment by
new mutex. This mutex is also held during ice_send_event_to_aux()
call to ensure that aux device is valid during that call. Device
lock used ice_send_event_to_aux() to avoid its concurrent run can
be removed as this is secured by that mutex.
Reproducer:
cycle=1
while :;do
echo "#### Cycle: $cycle"
ip link set ens7f0 mtu 9000
ip link add bond0 type bond mode 1 miimon 100
ip link set bond0 up
ifenslave bond0 ens7f0
ip link set bond0 mtu 9000
ethtool -L ens7f0 combined 1
ip link del bond0
ip link set ens7f0 mtu 1500
sleep 1
let cycle++
done
In short when the device is added/removed to/from bond the aux device
is unplugged/plugged. When MTU of the device is changed an event is
sent to aux device asynchronously. This can race with (un)plugging
operation and because pf->adev is set too early (plug) or too late
(unplug) the function ice_send_event_to_aux() can touch uninitialized
or destroyed fields. In the case of crash below pf->adev->dev.mutex.
Crash:
[ 53.372066] bond0: (slave ens7f0): making interface the new active one
[ 53.378622] bond0: (slave ens7f0): Enslaving as an active interface with an u
p link
[ 53.386294] IPv6: ADDRCONF(NETDEV_CHANGE): bond0: link becomes ready
[ 53.549104] bond0: (slave ens7f1): Enslaving as a backup interface with an up
link
[ 54.118906] ice 0000:ca:00.0 ens7f0: Number of in use tx queues changed inval
idating tc mappings. Priority traffic classification disabled!
[ 54.233374] ice 0000:ca:00.1 ens7f1: Number of in use tx queues changed inval
idating tc mappings. Priority traffic classification disabled!
[ 54.248204] bond0: (slave ens7f0): Releasing backup interface
[ 54.253955] bond0: (slave ens7f1): making interface the new active one
[ 54.274875] bond0: (slave ens7f1): Releasing backup interface
[ 54.289153] bond0 (unregistering): Released all slaves
[ 55.383179] MII link monitoring set to 100 ms
[ 55.398696] bond0: (slave ens7f0): making interface the new active one
[ 55.405241] BUG: kernel NULL pointer dereference, address: 0000000000000080
[ 55.405289] bond0: (slave ens7f0): Enslaving as an active interface with an u
p link
[ 55.412198] #PF: supervisor write access in kernel mode
[ 55.412200] #PF: error_code(0x0002) - not-present page
[ 55.412201] PGD 25d2ad067 P4D 0
[ 55.412204] Oops: 0002 [#1] PREEMPT SMP NOPTI
[ 55.412207] CPU: 0 PID: 403 Comm: kworker/0:2 Kdump: loaded Tainted: G S
5.17.0-13579-g57f2d6540f03 #1
[ 55.429094] bond0: (slave ens7f1): Enslaving as a backup interface with an up
link
[ 55.430224] Hardware name: Dell Inc. PowerEdge R750/06V45N, BIOS 1.4.4 10/07/
2021
[ 55.430226] Workqueue: ice ice_service_task [ice]
[ 55.468169] RIP: 0010:mutex_unlock+0x10/0x20
[ 55.472439] Code: 0f b1 13 74 96 eb e0 4c 89 ee eb d8 e8 79 54 ff ff 66 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 65 48 8b 04 25 40 ef 01 00 31 d2 <f0> 48 0f b1 17 75 01 c3 e9 e3 fe ff ff 0f 1f 00 0f 1f 44 00 00 48
[ 55.491186] RSP: 0018:ff4454230d7d7e28 EFLAGS: 00010246
[ 55.496413] RAX: ff1a79b208b08000 RBX: ff1a79b2182e8880 RCX: 0000000000000001
[ 55.503545] RDX: 0000000000000000 RSI: ff4454230d7d7db0 RDI: 0000000000000080
[ 55.510678] RBP: ff1a79d1c7e48b68 R08: ff4454230d7d7db0 R09: 0000000000000041
[ 55.517812] R10: 00000000000000a5 R11: 00000000000006e6 R12: ff1a79d1c7e48bc0
[ 55.524945] R13: 0000000000000000 R14: ff1a79d0ffc305c0 R15: 0000000000000000
[ 55.532076] FS: 0000000000000000(0000) GS:ff1a79d0ffc00000(0000) knlGS:0000000000000000
[ 55.540163] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 55.545908] CR2: 0000000000000080 CR3: 00000003487ae003 CR4: 0000000000771ef0
[ 55.553041] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 55.560173] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[ 55.567305] PKRU: 55555554
[ 55.570018] Call Trace:
[ 55.572474] <TASK>
[ 55.574579] ice_service_task+0xaab/0xef0 [ice]
[ 55.579130] process_one_work+0x1c5/0x390
[ 55.583141] ? process_one_work+0x390/0x390
[ 55.587326] worker_thread+0x30/0x360
[ 55.590994] ? process_one_work+0x390/0x390
[ 55.595180] kthread+0xe6/0x110
[ 55.598325] ? kthread_complete_and_exit+0x20/0x20
[ 55.603116] ret_from_fork+0x1f/0x30
[ 55.606698] </TASK>
Fixes: f9f5301e7e2d ("ice: Register auxiliary device to provide RDMA")
Cc: Leon Romanovsky <leon@kernel.org>
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
drivers/net/ethernet/intel/ice/ice.h | 1 +
drivers/net/ethernet/intel/ice/ice_idc.c | 33 ++++++++++++++---------
drivers/net/ethernet/intel/ice/ice_main.c | 2 ++
3 files changed, 23 insertions(+), 13 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h
index 8ed3c9ab7ff7..a895e3a8e988 100644
--- a/drivers/net/ethernet/intel/ice/ice.h
+++ b/drivers/net/ethernet/intel/ice/ice.h
@@ -540,6 +540,7 @@ struct ice_pf {
struct mutex avail_q_mutex; /* protects access to avail_[rx|tx]qs */
struct mutex sw_mutex; /* lock for protecting VSI alloc flow */
struct mutex tc_mutex; /* lock to protect TC changes */
+ struct mutex adev_mutex; /* lock to protect aux device access */
u32 msg_enable;
struct ice_ptp ptp;
struct tty_driver *ice_gnss_tty_driver;
diff --git a/drivers/net/ethernet/intel/ice/ice_idc.c b/drivers/net/ethernet/intel/ice/ice_idc.c
index 25a436d342c2..b9e471137f6a 100644
--- a/drivers/net/ethernet/intel/ice/ice_idc.c
+++ b/drivers/net/ethernet/intel/ice/ice_idc.c
@@ -10,13 +10,15 @@
* ice_get_auxiliary_drv - retrieve iidc_auxiliary_drv struct
* @pf: pointer to PF struct
*
- * This function has to be called with a device_lock on the
- * pf->adev.dev to avoid race conditions.
+ * This function has to be called with pf->adev_mutex held
+ * to avoid race conditions.
*/
static struct iidc_auxiliary_drv *ice_get_auxiliary_drv(struct ice_pf *pf)
{
struct auxiliary_device *adev;
+ lockdep_assert_held(&pf->adev_mutex);
+
adev = pf->adev;
if (!adev || !adev->dev.driver)
return NULL;
@@ -37,14 +39,13 @@ void ice_send_event_to_aux(struct ice_pf *pf, struct iidc_event *event)
if (WARN_ON_ONCE(!in_task()))
return;
- if (!pf->adev)
- return;
+ mutex_lock(&pf->adev_mutex);
- device_lock(&pf->adev->dev);
iadrv = ice_get_auxiliary_drv(pf);
if (iadrv && iadrv->event_handler)
iadrv->event_handler(pf, event);
- device_unlock(&pf->adev->dev);
+
+ mutex_unlock(&pf->adev_mutex);
}
/**
@@ -290,7 +291,6 @@ int ice_plug_aux_dev(struct ice_pf *pf)
return -ENOMEM;
adev = &iadev->adev;
- pf->adev = adev;
iadev->pf = pf;
adev->id = pf->aux_idx;
@@ -300,18 +300,20 @@ int ice_plug_aux_dev(struct ice_pf *pf)
ret = auxiliary_device_init(adev);
if (ret) {
- pf->adev = NULL;
kfree(iadev);
return ret;
}
ret = auxiliary_device_add(adev);
if (ret) {
- pf->adev = NULL;
auxiliary_device_uninit(adev);
return ret;
}
+ mutex_lock(&pf->adev_mutex);
+ pf->adev = adev;
+ mutex_unlock(&pf->adev_mutex);
+
return 0;
}
@@ -320,12 +322,17 @@ int ice_plug_aux_dev(struct ice_pf *pf)
*/
void ice_unplug_aux_dev(struct ice_pf *pf)
{
- if (!pf->adev)
- return;
+ struct auxiliary_device *adev;
- auxiliary_device_delete(pf->adev);
- auxiliary_device_uninit(pf->adev);
+ mutex_lock(&pf->adev_mutex);
+ adev = pf->adev;
pf->adev = NULL;
+ mutex_unlock(&pf->adev_mutex);
+
+ if (adev) {
+ auxiliary_device_delete(adev);
+ auxiliary_device_uninit(adev);
+ }
}
/**
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 5b1198859da7..2cbbf7abefc4 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -3769,6 +3769,7 @@ u16 ice_get_avail_rxq_count(struct ice_pf *pf)
static void ice_deinit_pf(struct ice_pf *pf)
{
ice_service_task_stop(pf);
+ mutex_destroy(&pf->adev_mutex);
mutex_destroy(&pf->sw_mutex);
mutex_destroy(&pf->tc_mutex);
mutex_destroy(&pf->avail_q_mutex);
@@ -3847,6 +3848,7 @@ static int ice_init_pf(struct ice_pf *pf)
mutex_init(&pf->sw_mutex);
mutex_init(&pf->tc_mutex);
+ mutex_init(&pf->adev_mutex);
INIT_HLIST_HEAD(&pf->aq_wait_list);
spin_lock_init(&pf->aq_wait_lock);
--
2.35.1
^ permalink raw reply related
* Re: [PATCH v2 2/3] net: phy: adin: add support for clock output
From: kernel test robot @ 2022-04-21 6:45 UTC (permalink / raw)
To: Josua Mayer, netdev
Cc: kbuild-all, alvaro.karsz, Josua Mayer, Michael Hennerich,
Andrew Lunn, Heiner Kallweit, Russell King, Jakub Kicinski,
Paolo Abeni
In-Reply-To: <20220419102709.26432-3-josua@solid-run.com>
Hi Josua,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on robh/for-next]
[also build test WARNING on net/master net-next/master v5.18-rc3 next-20220420]
[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]
url: https://github.com/intel-lab-lkp/linux/commits/Josua-Mayer/dt-bindings-net-adin-document-phy-clock-output-properties/20220419-192719
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: openrisc-randconfig-s032-20220420 (https://download.01.org/0day-ci/archive/20220421/202204211324.qgcPMycQ-lkp@intel.com/config)
compiler: or1k-linux-gcc (GCC) 11.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.4-dirty
# https://github.com/intel-lab-lkp/linux/commit/74d856f1c89a6534fd58889f20ad4b481b8191c9
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Josua-Mayer/dt-bindings-net-adin-document-phy-clock-output-properties/20220419-192719
git checkout 74d856f1c89a6534fd58889f20ad4b481b8191c9
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=openrisc SHELL=/bin/bash drivers/net/phy/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
>> drivers/net/phy/adin.c:448:27: sparse: sparse: Using plain integer as NULL pointer
vim +448 drivers/net/phy/adin.c
444
445 static int adin_config_clk_out(struct phy_device *phydev)
446 {
447 struct device *dev = &phydev->mdio.dev;
> 448 const char *val = 0;
449 u8 sel = 0;
450
451 device_property_read_string(dev, "adi,phy-output-clock", &val);
452 if(!val) {
453 /* property not present, do not enable GP_CLK pin */
454 } else if(strcmp(val, "25mhz-reference") == 0) {
455 sel |= ADIN1300_GE_CLK_CFG_25;
456 } else if(strcmp(val, "125mhz-free-running") == 0) {
457 sel |= ADIN1300_GE_CLK_CFG_FREE_125;
458 } else if(strcmp(val, "125mhz-recovered") == 0) {
459 sel |= ADIN1300_GE_CLK_CFG_RCVR_125;
460 } else if(strcmp(val, "adaptive-free-running") == 0) {
461 sel |= ADIN1300_GE_CLK_CFG_HRT_FREE;
462 } else if(strcmp(val, "adaptive-recovered") == 0) {
463 sel |= ADIN1300_GE_CLK_CFG_HRT_RCVR;
464 } else {
465 phydev_err(phydev, "invalid adi,phy-output-clock\n");
466 return -EINVAL;
467 }
468
469 if(device_property_read_bool(dev, "adi,phy-output-reference-clock"))
470 sel |= ADIN1300_GE_CLK_CFG_REF_EN;
471
472 return phy_modify_mmd(phydev, MDIO_MMD_VEND1, ADIN1300_GE_CLK_CFG_REG,
473 ADIN1300_GE_CLK_CFG_MASK, sel);
474 }
475
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply
* Re: [PATCHv2 bpf-next 3/4] bpf: Resolve symbols with kallsyms_lookup_names for kprobe multi link
From: Jiri Olsa @ 2022-04-21 6:49 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Masami Hiramatsu, Networking, bpf, lkml, Martin KaFai Lau,
Song Liu, Yonghong Song, John Fastabend, KP Singh
In-Reply-To: <CAEf4Bzau_RmREQwVQ6wPRbCHVXRuAr1k08btaft2jUwBYTeM-Q@mail.gmail.com>
On Wed, Apr 20, 2022 at 02:49:59PM -0700, Andrii Nakryiko wrote:
> On Mon, Apr 18, 2022 at 5:49 AM Jiri Olsa <jolsa@kernel.org> wrote:
> >
> > Using kallsyms_lookup_names function to speed up symbols lookup in
> > kprobe multi link attachment and replacing with it the current
> > kprobe_multi_resolve_syms function.
> >
> > This speeds up bpftrace kprobe attachment:
> >
> > # perf stat -r 5 -e cycles ./src/bpftrace -e 'kprobe:x* { } i:ms:1 { exit(); }'
> > ...
> > 6.5681 +- 0.0225 seconds time elapsed ( +- 0.34% )
> >
> > After:
> >
> > # perf stat -r 5 -e cycles ./src/bpftrace -e 'kprobe:x* { } i:ms:1 { exit(); }'
> > ...
> > 0.5661 +- 0.0275 seconds time elapsed ( +- 4.85% )
> >
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
>
> LGTM.
>
> Acked-by: Andrii Nakryiko <andrii@kernel.org>
>
> > kernel/trace/bpf_trace.c | 113 +++++++++++++++++++++++----------------
> > 1 file changed, 67 insertions(+), 46 deletions(-)
> >
> > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> > index b26f3da943de..f49cdc46a21f 100644
> > --- a/kernel/trace/bpf_trace.c
> > +++ b/kernel/trace/bpf_trace.c
> > @@ -2226,6 +2226,60 @@ struct bpf_kprobe_multi_run_ctx {
> > unsigned long entry_ip;
> > };
> >
> > +struct user_syms {
> > + const char **syms;
> > + char *buf;
> > +};
> > +
> > +static int copy_user_syms(struct user_syms *us, unsigned long __user *usyms, u32 cnt)
> > +{
> > + unsigned long __user usymbol;
> > + const char **syms = NULL;
> > + char *buf = NULL, *p;
> > + int err = -EFAULT;
> > + unsigned int i;
> > +
> > + err = -ENOMEM;
> > + syms = kvmalloc(cnt * sizeof(*syms), GFP_KERNEL);
> > + if (!syms)
> > + goto error;
> > +
> > + buf = kvmalloc(cnt * KSYM_NAME_LEN, GFP_KERNEL);
> > + if (!buf)
> > + goto error;
> > +
> > + for (p = buf, i = 0; i < cnt; i++) {
> > + if (__get_user(usymbol, usyms + i)) {
> > + err = -EFAULT;
> > + goto error;
> > + }
> > + err = strncpy_from_user(p, (const char __user *) usymbol, KSYM_NAME_LEN);
> > + if (err == KSYM_NAME_LEN)
> > + err = -E2BIG;
> > + if (err < 0)
> > + goto error;
> > + syms[i] = p;
> > + p += err + 1;
> > + }
> > +
> > + err = 0;
> > + us->syms = syms;
> > + us->buf = buf;
>
> return 0 here instead of falling through into error: block?
ok, will change
jirka
>
> > +
> > +error:
> > + if (err) {
> > + kvfree(syms);
> > + kvfree(buf);
> > + }
> > + return err;
> > +}
> > +
> > +static void free_user_syms(struct user_syms *us)
> > +{
> > + kvfree(us->syms);
> > + kvfree(us->buf);
> > +}
> > +
> > static void bpf_kprobe_multi_link_release(struct bpf_link *link)
> > {
> > struct bpf_kprobe_multi_link *kmulti_link;
>
> [...]
^ permalink raw reply
* Re: [PATCH v5 1/3] selftests: bpf: add test for bpf_skb_change_proto
From: Lina.Wang @ 2022-04-21 6:49 UTC (permalink / raw)
To: Daniel Borkmann, David S . Miller, Jakub Kicinski, Paolo Abeni,
Shuah Khan, Matthias Brugger
Cc: Alexei Starovoitov, Andrii Nakryiko, Jesper Dangaard Brouer,
Eric Dumazet, linux-kernel, Maciej enczykowski, linux-kselftest,
netdev, bpf
In-Reply-To: <9dc51533-92d2-1c82-2a6e-96e1ac747bb7@iogearbox.net>
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="y", Size: 959 bytes --]
On Thu, 2022-04-07 at 17:22 +0200, Daniel Borkmann wrote:
> Hi Lina,
>
> On 4/7/22 10:47 AM, Lina Wang wrote:
> > The code is copied from the Android Open Source Project and the
> > author(
> > Maciej Żenczykowski) has gave permission to relicense it under
> > GPLv2.
> >
> > The test is to change input IPv6 packets to IPv4 ones and output
> > IPv4 to
> > IPv6 with bpf_skb_change_proto.
> > ---
>
> Your patch 2/3 is utilizing this program out of
> selftests/net/udpgro_frglist.sh,
> however, this is a bit problematic given BPF CI which runs on every
> BPF submitted
> patch. Meaning, udpgro_frglist.sh won't be covered by CI and only
> needs to be run
> manually. Could you properly include this into test_progs from BPF
> suite (that way,
> BPF CI will also pick it up)? See also [2] for more complex netns
Please check my previous response, do you agree with me? I can move such nat6to4.c to net/, not bpf/, no need to add bpf test progs
Thanks!
^ permalink raw reply
* Re: [PATCHv2 bpf-next 4/4] selftests/bpf: Add attach bench test
From: Jiri Olsa @ 2022-04-21 6:56 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Masami Hiramatsu, Networking, bpf, lkml, Martin KaFai Lau,
Song Liu, Yonghong Song, John Fastabend, KP Singh
In-Reply-To: <CAEf4BzYuvLgVtxbtz7pjCmtSp0hEKJd0peCnbX0E_-Tqy5g4dw@mail.gmail.com>
On Wed, Apr 20, 2022 at 02:56:53PM -0700, Andrii Nakryiko wrote:
SNIP
> > +#define DEBUGFS "/sys/kernel/debug/tracing/"
> > +
> > +static int get_syms(char ***symsp, size_t *cntp)
> > +{
> > + size_t cap = 0, cnt = 0, i;
> > + char *name, **syms = NULL;
> > + struct hashmap *map;
> > + char buf[256];
> > + FILE *f;
> > + int err;
> > +
> > + /*
> > + * The available_filter_functions contains many duplicates,
> > + * but other than that all symbols are usable in kprobe multi
> > + * interface.
> > + * Filtering out duplicates by using hashmap__add, which won't
> > + * add existing entry.
> > + */
> > + f = fopen(DEBUGFS "available_filter_functions", "r");
>
> nit: DEBUGFS "constant" just makes it harder to follow the code and
> doesn't add anything, please just use the full path here directly
there's another one DEBUGFS in trace_helpers.c,
we could put it in trace_helpers.h
>
> > + if (!f)
> > + return -EINVAL;
> > +
> > + map = hashmap__new(symbol_hash, symbol_equal, NULL);
> > + err = libbpf_get_error(map);
>
> hashmap__new() is an internal API, so please use IS_ERR() directly
> here. libbpf_get_error() should be used for public libbpf APIs, and
> preferably not in libbpf 1.0 mode
ok
>
> > + if (err)
> > + goto error;
> > +
> > + while (fgets(buf, sizeof(buf), f)) {
> > + /* skip modules */
> > + if (strchr(buf, '['))
> > + continue;
>
> [...]
>
> > + attach_delta = (attach_end_ns - attach_start_ns) / 1000000000.0;
> > + detach_delta = (detach_end_ns - detach_start_ns) / 1000000000.0;
> > +
> > + fprintf(stderr, "%s: found %lu functions\n", __func__, cnt);
> > + fprintf(stderr, "%s: attached in %7.3lfs\n", __func__, attach_delta);
> > + fprintf(stderr, "%s: detached in %7.3lfs\n", __func__, detach_delta);
>
>
> why stderr? just do printf() and let test_progs handle output
ok
>
>
> > +
> > +cleanup:
> > + kprobe_multi_empty__destroy(skel);
> > + if (syms) {
> > + for (i = 0; i < cnt; i++)
> > + free(syms[i]);
> > + free(syms);
> > + }
> > +}
> > +
> > void test_kprobe_multi_test(void)
> > {
> > if (!ASSERT_OK(load_kallsyms(), "load_kallsyms"))
> > @@ -320,4 +454,6 @@ void test_kprobe_multi_test(void)
> > test_attach_api_syms();
> > if (test__start_subtest("attach_api_fails"))
> > test_attach_api_fails();
> > + if (test__start_subtest("bench_attach"))
> > + test_bench_attach();
> > }
> > diff --git a/tools/testing/selftests/bpf/progs/kprobe_multi_empty.c b/tools/testing/selftests/bpf/progs/kprobe_multi_empty.c
> > new file mode 100644
> > index 000000000000..be9e3d891d46
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/progs/kprobe_multi_empty.c
> > @@ -0,0 +1,12 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +#include <linux/bpf.h>
> > +#include <bpf/bpf_helpers.h>
> > +#include <bpf/bpf_tracing.h>
> > +
> > +char _license[] SEC("license") = "GPL";
> > +
> > +SEC("kprobe.multi/*")
>
> use SEC("kprobe.multi") to make it clear that we are attaching it "manually"?
yep, will do
thanks,
jirka
^ permalink raw reply
* [PATCH] net: unexport csum_and_copy_{from,to}_user
From: Christoph Hellwig @ 2022-04-21 7:04 UTC (permalink / raw)
To: akpm; +Cc: x86, linux-alpha, linux-m68k, linuxppc-dev, linux-kernel, netdev
csum_and_copy_from_user and csum_and_copy_to_user are exported by
a few architectures, but not actually used in modular code. Drop
the exports.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/alpha/lib/csum_partial_copy.c | 1 -
arch/m68k/lib/checksum.c | 2 --
arch/powerpc/lib/checksum_wrappers.c | 2 --
arch/x86/lib/csum-wrappers_64.c | 2 --
4 files changed, 7 deletions(-)
diff --git a/arch/alpha/lib/csum_partial_copy.c b/arch/alpha/lib/csum_partial_copy.c
index 1931a04af85a2..4d180d96f09e4 100644
--- a/arch/alpha/lib/csum_partial_copy.c
+++ b/arch/alpha/lib/csum_partial_copy.c
@@ -353,7 +353,6 @@ csum_and_copy_from_user(const void __user *src, void *dst, int len)
return 0;
return __csum_and_copy(src, dst, len);
}
-EXPORT_SYMBOL(csum_and_copy_from_user);
__wsum
csum_partial_copy_nocheck(const void *src, void *dst, int len)
diff --git a/arch/m68k/lib/checksum.c b/arch/m68k/lib/checksum.c
index 7e6afeae62177..5acb821849d30 100644
--- a/arch/m68k/lib/checksum.c
+++ b/arch/m68k/lib/checksum.c
@@ -265,8 +265,6 @@ csum_and_copy_from_user(const void __user *src, void *dst, int len)
return sum;
}
-EXPORT_SYMBOL(csum_and_copy_from_user);
-
/*
* copy from kernel space while checksumming, otherwise like csum_partial
diff --git a/arch/powerpc/lib/checksum_wrappers.c b/arch/powerpc/lib/checksum_wrappers.c
index f3999cbb2fcc4..1a14c8780278c 100644
--- a/arch/powerpc/lib/checksum_wrappers.c
+++ b/arch/powerpc/lib/checksum_wrappers.c
@@ -24,7 +24,6 @@ __wsum csum_and_copy_from_user(const void __user *src, void *dst,
user_read_access_end();
return csum;
}
-EXPORT_SYMBOL(csum_and_copy_from_user);
__wsum csum_and_copy_to_user(const void *src, void __user *dst, int len)
{
@@ -38,4 +37,3 @@ __wsum csum_and_copy_to_user(const void *src, void __user *dst, int len)
user_write_access_end();
return csum;
}
-EXPORT_SYMBOL(csum_and_copy_to_user);
diff --git a/arch/x86/lib/csum-wrappers_64.c b/arch/x86/lib/csum-wrappers_64.c
index 189344924a2be..145f9a0bde29a 100644
--- a/arch/x86/lib/csum-wrappers_64.c
+++ b/arch/x86/lib/csum-wrappers_64.c
@@ -32,7 +32,6 @@ csum_and_copy_from_user(const void __user *src, void *dst, int len)
user_access_end();
return sum;
}
-EXPORT_SYMBOL(csum_and_copy_from_user);
/**
* csum_and_copy_to_user - Copy and checksum to user space.
@@ -57,7 +56,6 @@ csum_and_copy_to_user(const void *src, void __user *dst, int len)
user_access_end();
return sum;
}
-EXPORT_SYMBOL(csum_and_copy_to_user);
/**
* csum_partial_copy_nocheck - Copy and checksum.
--
2.30.2
^ permalink raw reply related
* linux-next: build warning after merge of the net-next tree
From: Stephen Rothwell @ 2022-04-21 7:07 UTC (permalink / raw)
To: David Miller, Networking
Cc: Marc Kleine-Budde, Martin Jerabek, Ondrej Ille, Pavel Pisa,
Linux Kernel Mailing List, Linux Next Mailing List
[-- Attachment #1: Type: text/plain, Size: 367 bytes --]
Hi all,
After merging the net-next tree, today's linux-next build (htmldocs)
produced this warning:
Documentation/networking/device_drivers/can/ctu/ctucanfd-driver.rst: WARNING: document isn't included in any toctree
Introduced by commit
c3a0addefbde ("docs: ctucanfd: CTU CAN FD open-source IP core documentation.")
--
Cheers,
Stephen Rothwell
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH RFC 1/5] net: Add distinct sk_psock field
From: Hannes Reinecke @ 2022-04-21 7:35 UTC (permalink / raw)
To: Chuck Lever, netdev, linux-nfs, linux-nvme, linux-cifs,
linux-fsdevel
Cc: ak, borisp, simo
In-Reply-To: <165030056960.5073.6664402939918720250.stgit@oracle-102.nfsv4.dev>
On 4/18/22 18:49, Chuck Lever wrote:
> The sk_psock facility populates the sk_user_data field with the
> address of an extra bit of metadata. User space sockets never
> populate the sk_user_data field, so this has worked out fine.
>
> However, kernel consumers such as the RPC client and server do
> populate the sk_user_data field. The sk_psock() function cannot tell
> that the content of sk_user_data does not point to psock metadata,
> so it will happily return a pointer to something else, cast to a
> struct sk_psock.
>
> Thus kernel consumers and psock currently cannot co-exist.
>
> We could educate sk_psock() to return NULL if sk_user_data does
> not point to a struct sk_psock. However, a more general solution
> that enables full co-existence psock and other uses of sk_user_data
> might be more interesting.
>
> Move the struct sk_psock address to its own pointer field so that
> the contents of the sk_user_data field is preserved.
>
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
> include/linux/skmsg.h | 2 +-
> include/net/sock.h | 4 +++-
> net/core/skmsg.c | 6 +++---
> 3 files changed, 7 insertions(+), 5 deletions(-)
>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer
^ permalink raw reply
* Re: [PATCH RFC 3/5] net/tls: Add an AF_TLSH address family
From: Hannes Reinecke @ 2022-04-21 7:35 UTC (permalink / raw)
To: Chuck Lever, netdev, linux-nfs, linux-nvme, linux-cifs,
linux-fsdevel
Cc: ak, borisp, simo
In-Reply-To: <165030058340.5073.5461321687798728373.stgit@oracle-102.nfsv4.dev>
On 4/18/22 18:49, Chuck Lever wrote:
> Add definitions for an AF_TLSH address family. The next patch
> explains its purpose and operation.
>
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
> include/linux/socket.h | 4 +++-
> net/core/sock.c | 2 +-
> net/socket.c | 1 +
> security/selinux/hooks.c | 4 +++-
> security/selinux/include/classmap.h | 4 +++-
> tools/perf/trace/beauty/include/linux/socket.h | 4 +++-
> 6 files changed, 14 insertions(+), 5 deletions(-)
>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer
^ permalink raw reply
* Re: [PATCH RFC 4/5] net/tls: Add support for PF_TLSH (a TLS handshake listener)
From: Hannes Reinecke @ 2022-04-21 7:36 UTC (permalink / raw)
To: Chuck Lever, netdev, linux-nfs, linux-nvme, linux-cifs,
linux-fsdevel
Cc: ak, borisp, simo
In-Reply-To: <165030059051.5073.16723746870370826608.stgit@oracle-102.nfsv4.dev>
On 4/18/22 18:49, Chuck Lever wrote:
> In-kernel TLS consumers need a way to perform a TLS handshake. In
> the absence of a handshake implementation in the kernel itself, a
> mechanism to perform the handshake in user space, using an existing
> TLS handshake library, is necessary.
>
> I've designed a way to pass a connected kernel socket endpoint to
> user space using the traditional listen/accept mechanism. accept(2)
> gives us a well-understood way to materialize a socket endpoint as a
> normal file descriptor in a specific user space process. Like any
> open socket descriptor, the accepted FD can then be passed to a
> library such as openSSL to perform a TLS handshake.
>
> This prototype currently handles only initiating client-side TLS
> handshakes. Server-side handshakes and key renegotiation are left
> to do.
>
> Security Considerations
> ~~~~~~~~ ~~~~~~~~~~~~~~
>
> This prototype is net-namespace aware.
>
> The kernel has no mechanism to attest that the listening user space
> agent is trustworthy.
>
> Currently the prototype does not handle multiple listeners that
> overlap -- multiple listeners in the same net namespace that have
> overlapping bind addresses.
>
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
> .../networking/tls-in-kernel-handshake.rst | 103 ++
> include/linux/socket.h | 1
> include/net/sock.h | 3
> include/net/tls.h | 15
> include/net/tlsh.h | 22
> include/uapi/linux/tls.h | 16
> net/core/sock.c | 2
> net/tls/Makefile | 2
> net/tls/af_tlsh.c | 1040 ++++++++++++++++++++
> net/tls/tls_main.c | 10
> 10 files changed, 1213 insertions(+), 1 deletion(-)
> create mode 100644 Documentation/networking/tls-in-kernel-handshake.rst
> create mode 100644 include/net/tlsh.h
> create mode 100644 net/tls/af_tlsh.c
>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer
^ permalink raw reply
* Re: [PATCH RFC 5/5] net/tls: Add observability for AF_TLSH sockets
From: Hannes Reinecke @ 2022-04-21 7:36 UTC (permalink / raw)
To: Chuck Lever, netdev, linux-nfs, linux-nvme, linux-cifs,
linux-fsdevel
Cc: ak, borisp, simo
In-Reply-To: <165030059773.5073.6168640435213548957.stgit@oracle-102.nfsv4.dev>
On 4/18/22 18:49, Chuck Lever wrote:
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
> net/tls/af_tlsh.c | 50 +++++++
> net/tls/trace.c | 3
> net/tls/trace.h | 355 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 402 insertions(+), 6 deletions(-)
>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer
^ permalink raw reply
* Re: [PATCH net-next 03/12] dt-bindings: net: pcs: add bindings for Renesas RZ/N1 MII converter
From: Clément Léger @ 2022-04-21 7:35 UTC (permalink / raw)
To: Rob Herring
Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean,
David S . Miller, Jakub Kicinski, Paolo Abeni,
Krzysztof Kozlowski, Geert Uytterhoeven, Magnus Damm,
Heiner Kallweit, Russell King, Thomas Petazzoni, Herve Codina,
Miquèl Raynal, Milan Stevanovic, Jimmy Lalande, linux-kernel,
devicetree, linux-renesas-soc, netdev
In-Reply-To: <YmBo7sj+PXoJaqo8@robh.at.kernel.org>
Le Wed, 20 Apr 2022 15:11:26 -0500,
Rob Herring <robh@kernel.org> a écrit :
> On Tue, Apr 19, 2022 at 05:00:44PM +0200, Clément Léger wrote:
> > Le Tue, 19 Apr 2022 08:43:47 -0500,
> > Rob Herring <robh@kernel.org> a écrit :
> >
> > > > + clocks:
> > > > + items:
> > > > + - description: MII reference clock
> > > > + - description: RGMII reference clock
> > > > + - description: RMII reference clock
> > > > + - description: AHB clock used for the MII converter register interface
> > > > +
> > > > + renesas,miic-cfg-mode:
> > > > + description: MII mux configuration mode. This value should use one of the
> > > > + value defined in dt-bindings/net/pcs-rzn1-miic.h.
> > >
> > > Describe possible values here as constraints. At present, I don't see
> > > the point of this property if there is only 1 possible value and it is
> > > required.
> >
> > The ethernet subsystem contains a number of internal muxes that allows
> > to configure ethernet routing. This configuration option allows to set
> > the register that configure these muxes.
> >
> > After talking with Andrew, I considered moving to something like this:
> >
> > eth-miic@44030000 {
> > compatible = "renesas,rzn1-miic";
> >
> > mii_conv1: mii-conv-1 {
> > renesas,miic-input = <MIIC_GMAC1_PORT>;
> > port = <1>;
>
> 'port' is already used, find another name. Maybe 'reg' here. Don't know.
> What do 1 and 2 represent?
'port' represent the port index in the MII converter IP. I went with reg
first, but according to Andrew feedback, it looked like it was a bad
idea since it was not really a "bus". However, this pattern is already
used for dsa ports.
> > >
> > > Why do you need sub-nodes? They don't have any properties. A simple mask
> > > property could tell you which ports are present/active/enabled if that's
> > > what you are tracking. Or the SoC specific compatibles you need to add
> > > can imply the ports if they are SoC specific.
> >
> > The MACs are using phandles to these sub-nodes to query a specific MII
> > converter port PCS:
> >
> > switch@44050000 {
> > compatible = "renesas,rzn1-a5psw";
> >
> > ports {
> > port@0 {
>
> ethernet-ports and ethernet-port so we don't collide with the graph
> binding.
Acked.
>
>
> > reg = <0>;
> > label = "lan0";
> > phy-handle = <&switch0phy3>;
> > pcs-handle = <&mii_conv4>;
> > };
> > };
> > };
> >
> > According to Andrew, this is not a good idea to represent the PCS as a
> > bus since it is indeed not a bus. I could also switch to something like
> > pcs-handle = <ð_mii 4> but i'm not sure what you'd prefer. We could
> > also remove this from the device-tree and consider each driver to
> > request the MII ouput to be configured using something like this for
> > instance:
>
> I'm pretty sure we already defined pcs-handle as only a phandle. If you
> want variable cells, then it's got to be extended like all the other
> properties using that pattern.
Yep, it seems to be used in some other driver as a single phandle too.
I'll kept that.
>
> >
> > miic_request_pcs(pcs_np, miic_port_nr, MIIC_SWITCHD_PORT);
> >
> > But I'm not really fan of this because it requires the drivers to
> > assume some specificities of the MII converter (port number are not in
> > the same order of the switch for instance) and thus I would prefer this
> > to be in the device-tree.
> >
> > Let me know if you can think of something that would suit you
> > better but keep in mind that I need to correctly match a switch/MAC
> > port with a PCS port and that I also need to configure MII internal
> > muxes.
> >
> > For more information, you can look at section 8 of the manual at [1].
>
> I can't really. Other people want their bindings reviewed too.
No worries.
>
> Rob
--
Clément Léger,
Embedded Linux and Kernel engineer at Bootlin
https://bootlin.com
^ permalink raw reply
* Re: linux-next: build warning after merge of the net-next tree
From: Pavel Pisa @ 2022-04-21 7:29 UTC (permalink / raw)
To: Stephen Rothwell
Cc: David Miller, Networking, Marc Kleine-Budde, Martin Jerabek,
Ondrej Ille, Linux Kernel Mailing List, Linux Next Mailing List
In-Reply-To: <20220421170749.1c0b56db@canb.auug.org.au>
Hello Stephen,
thanks for the notice.
On Thursday 21 of April 2022 09:07:49 Stephen Rothwell wrote:
> After merging the net-next tree, today's linux-next build (htmldocs)
> produced this warning:
>
> Documentation/networking/device_drivers/can/ctu/ctucanfd-driver.rst:
> WARNING: document isn't included in any toctree
>
> Introduced by commit
>
> c3a0addefbde ("docs: ctucanfd: CTU CAN FD open-source IP core
> documentation.")
I would be happy for suggestion for reference location.
Is the next file right location
Documentation/networking/device_drivers/can/index.rst
for reference to
Documentation/networking/device_drivers/can/ctu/ctucanfd-driver.rst
It is documentation of the driver for CAN FD open-source
IP core developed by the group formed at Czech Technical University.
I have probably minor updates for the links to the external
resources, AXI, APB and other documentation because
it moves from site to site under Intel, ARM, Xilinx
web sites hierarchies.
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 net-next 08/12] net: dsa: rzn1-a5psw: add FDB support
From: Clément Léger @ 2022-04-21 7:38 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, David S . Miller,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Geert Uytterhoeven, Magnus Damm, Heiner Kallweit, Russell King,
Thomas Petazzoni, Herve Codina, Miquèl Raynal,
Milan Stevanovic, Jimmy Lalande, linux-kernel, devicetree,
linux-renesas-soc, netdev
In-Reply-To: <20220420195214.dnekbfhha53trbke@skbuf>
Le Wed, 20 Apr 2022 22:52:14 +0300,
Vladimir Oltean <olteanv@gmail.com> a écrit :
> > >
> > > Shouldn't this contain something along the lines of a VID, FID, something?
> >
> > This is extracted directly from the datasheet [1]. The switch FDB table
> > does not seems to store the VID with the entries (See page 300).
> >
> > [1]
> > https://www.renesas.com/us/en/document/mah/rzn1d-group-rzn1s-group-rzn1l-group-users-manual-r-engine-and-ethernet-peripherals
>
> Thanks for the link. I see that the switch has a non-partitionable
> lookup table, not even by VLAN. A shame.
>
> This is also in contrast with the software bridge driver, where FDB and
> MDB entries can have independent destinations per VID.
>
> So there's nothing you can do beyond limiting to a single offloaded
> bridge and hoping for the best w.r.t. per-VLAN forwarding destinations.
>
> Note that if you limit to a single bridge does not mean that you can
> declare ds->fdb_isolation = true. Declaring that would opt you into
> unicast and multicast filtering towards the CPU, i.o.w. a method for
> software to only receive the addresses it has expressed an interest in,
> rather than all packets received on standalone ports. The way that is
> implemented in DSA is by adding FDB and MDB entries on the management
> port, and it would break a lot of things without a partitioning scheme
> for the lookup table.
Thanks Vladimir, it confirms what I thought.
--
Clément Léger,
Embedded Linux and Kernel engineer at Bootlin
https://bootlin.com
^ permalink raw reply
* Re: linux-next: build warning after merge of the net-next tree
From: Marc Kleine-Budde @ 2022-04-21 7:39 UTC (permalink / raw)
To: Pavel Pisa
Cc: Stephen Rothwell, David Miller, Networking, Martin Jerabek,
Ondrej Ille, Linux Kernel Mailing List, Linux Next Mailing List
In-Reply-To: <202204210929.46477.pisa@cmp.felk.cvut.cz>
[-- Attachment #1: Type: text/plain, Size: 1067 bytes --]
On 21.04.2022 09:29:46, Pavel Pisa wrote:
> On Thursday 21 of April 2022 09:07:49 Stephen Rothwell wrote:
> > After merging the net-next tree, today's linux-next build (htmldocs)
> > produced this warning:
> >
> > Documentation/networking/device_drivers/can/ctu/ctucanfd-driver.rst:
> > WARNING: document isn't included in any toctree
> >
> > Introduced by commit
> >
> > c3a0addefbde ("docs: ctucanfd: CTU CAN FD open-source IP core
> > documentation.")
>
> I would be happy for suggestion for reference location.
>
> Is the next file right location
>
> Documentation/networking/device_drivers/can/index.rst
>
> for reference to
>
> Documentation/networking/device_drivers/can/ctu/ctucanfd-driver.rst
Feel free to send a patch. I'm happy to take it.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung West/Dortmund | Phone: +49-231-2826-924 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* RE: [PATCH v2 bpf 07/11] samples/bpf: fix uin64_t format literals
From: David Laight @ 2022-04-21 7:46 UTC (permalink / raw)
To: 'Alexander Lobakin', Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko
Cc: Maciej Fijalkowski, Song Liu, Kumar Kartikeya Dwivedi,
bpf@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20220421003152.339542-8-alobakin@pm.me>
From: Alexander Lobakin
> Sent: 21 April 2022 01:40
>
> There's a couple places where uin64_t is being passed as an %lu
> format argument. That type is defined as unsigned long on 64-bit
> systems and as unsigned long long on 32-bit, so neither %lu nor
> %llu are not universal.
> One of the options is %PRIu64, but since it's always 8-byte long,
> just cast it to the _proper_ __u64 and print as %llu.
Is __u64 guaranteed to be 'unsigned long long' ? No reason why it should be.
I think you need to cast to (unsigned long long).
David
> Fixes: 51570a5ab2b7 ("A Sample of using socket cookie and uid for traffic monitoring")
> Fixes: 00f660eaf378 ("Sample program using SO_COOKIE")
> Signed-off-by: Alexander Lobakin <alobakin@pm.me>
> ---
> samples/bpf/cookie_uid_helper_example.c | 12 ++++++------
> samples/bpf/lwt_len_hist_user.c | 7 ++++---
> 2 files changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/samples/bpf/cookie_uid_helper_example.c b/samples/bpf/cookie_uid_helper_example.c
> index f0df3dda4b1f..269fac58fd5c 100644
> --- a/samples/bpf/cookie_uid_helper_example.c
> +++ b/samples/bpf/cookie_uid_helper_example.c
> @@ -207,9 +207,9 @@ static void print_table(void)
> error(1, errno, "fail to get entry value of Key: %u\n",
> curN);
> } else {
> - printf("cookie: %u, uid: 0x%x, Packet Count: %lu,"
> - " Bytes Count: %lu\n", curN, curEntry.uid,
> - curEntry.packets, curEntry.bytes);
> + printf("cookie: %u, uid: 0x%x, Packet Count: %llu, Bytes Count: %llu\n",
> + curN, curEntry.uid, (__u64)curEntry.packets,
> + (__u64)curEntry.bytes);
> }
> }
> }
> @@ -265,9 +265,9 @@ static void udp_client(void)
> if (res < 0)
> error(1, errno, "lookup sk stat failed, cookie: %lu\n",
> cookie);
> - printf("cookie: %lu, uid: 0x%x, Packet Count: %lu,"
> - " Bytes Count: %lu\n\n", cookie, dataEntry.uid,
> - dataEntry.packets, dataEntry.bytes);
> + printf("cookie: %llu, uid: 0x%x, Packet Count: %llu, Bytes Count: %llu\n\n",
> + (__u64)cookie, dataEntry.uid, (__u64)dataEntry.packets,
> + (__u64)dataEntry.bytes);
> }
> close(s_send);
> close(s_rcv);
> diff --git a/samples/bpf/lwt_len_hist_user.c b/samples/bpf/lwt_len_hist_user.c
> index 430a4b7e353e..c682faa75a2b 100644
> --- a/samples/bpf/lwt_len_hist_user.c
> +++ b/samples/bpf/lwt_len_hist_user.c
> @@ -44,7 +44,8 @@ int main(int argc, char **argv)
>
> while (bpf_map_get_next_key(map_fd, &key, &next_key) == 0) {
> if (next_key >= MAX_INDEX) {
> - fprintf(stderr, "Key %lu out of bounds\n", next_key);
> + fprintf(stderr, "Key %llu out of bounds\n",
> + (__u64)next_key);
> continue;
> }
>
> @@ -66,8 +67,8 @@ int main(int argc, char **argv)
>
> for (i = 1; i <= max_key + 1; i++) {
> stars(starstr, data[i - 1], max_value, MAX_STARS);
> - printf("%8ld -> %-8ld : %-8ld |%-*s|\n",
> - (1l << i) >> 1, (1l << i) - 1, data[i - 1],
> + printf("%8ld -> %-8ld : %-8lld |%-*s|\n",
> + (1l << i) >> 1, (1l << i) - 1, (__u64)data[i - 1],
> MAX_STARS, starstr);
> }
>
> --
> 2.36.0
>
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply
* Re: [PATCH] net: linkwatch: ignore events for unregistered netdevs
From: Paolo Abeni @ 2022-04-21 8:02 UTC (permalink / raw)
To: Lukas Wunner, Oliver Neukum, David S. Miller, Jakub Kicinski,
Jann Horn, Oleksij Rempel, Eric Dumazet
Cc: netdev, linux-usb, Andrew Lunn, Jacky Chou, Willy Tarreau,
Lino Sanfilippo, Philipp Rosenberger, Heiner Kallweit,
Greg Kroah-Hartman
In-Reply-To: <18b3541e5372bc9b9fc733d422f4e698c089077c.1650177997.git.lukas@wunner.de>
On Sun, 2022-04-17 at 09:04 +0200, Lukas Wunner wrote:
> Jann Horn reports a use-after-free on disconnect of a USB Ethernet
> (ax88179_178a.c). Oleksij Rempel has witnessed the same issue with a
> different driver (ax88172a.c).
>
> Jann's report (linked below) explains the root cause in great detail,
> but the gist is that USB Ethernet drivers call linkwatch_fire_event()
> between unregister_netdev() and free_netdev(). The asynchronous work
> linkwatch_event() may thus access the netdev after it's been freed.
>
> USB Ethernet may not even be the only culprit. To address the problem
> in the most general way, ignore link events once a netdev's state has
> been set to NETREG_UNREGISTERED.
>
> That happens in netdev_run_todo() immediately before the call to
> linkwatch_forget_dev(). Note that lweventlist_lock (and its implied
> memory barrier) guarantees that a linkwatch_add_event() running after
> linkwatch_forget_dev() will see the netdev's new state and bail out.
> An unregistered netdev is therefore never added to link_watch_list
> (but may have its __LINK_STATE_LINKWATCH_PENDING bit set, which should
> not matter). That obviates the need to invoke linkwatch_run_queue() in
> netdev_wait_allrefs(), so drop it.
>
> In a sense, the present commit is to *no longer* registered netdevs as
> commit b47300168e77 ("net: Do not fire linkwatch events until the device
> is registered.") is to *not yet* registered netdevs.
>
> Reported-by: Jann Horn <jannh@google.com>
> Link: https://lore.kernel.org/netdev/CAG48ez0MHBbENX5gCdHAUXZ7h7s20LnepBF-pa5M=7Bi-jZrEA@mail.gmail.com/
> Reported-by: Oleksij Rempel <o.rempel@pengutronix.de>
> Link: https://lore.kernel.org/netdev/20220315113841.GA22337@pengutronix.de/
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> Cc: stable@vger.kernel.org
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Oliver Neukum <oneukum@suse.com>
> Cc: Andrew Lunn <andrew@lunn.ch>
> ---
> include/linux/netdevice.h | 2 --
> net/core/dev.c | 17 -----------------
> net/core/link_watch.c | 10 ++--------
> 3 files changed, 2 insertions(+), 27 deletions(-)
>
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 59e27a2b7bf0..5d950b45b59d 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -4805,8 +4805,6 @@ extern const struct kobj_ns_type_operations net_ns_type_operations;
>
> const char *netdev_drivername(const struct net_device *dev);
>
> -void linkwatch_run_queue(void);
> -
> static inline netdev_features_t netdev_intersect_features(netdev_features_t f1,
> netdev_features_t f2)
> {
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 8c6c08446556..0ee56965ff76 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -10140,23 +10140,6 @@ static struct net_device *netdev_wait_allrefs_any(struct list_head *list)
> list_for_each_entry(dev, list, todo_list)
> call_netdevice_notifiers(NETDEV_UNREGISTER, dev);
>
> - __rtnl_unlock();
> - rcu_barrier();
> - rtnl_lock();
> -
> - list_for_each_entry(dev, list, todo_list)
> - if (test_bit(__LINK_STATE_LINKWATCH_PENDING,
> - &dev->state)) {
> - /* We must not have linkwatch events
> - * pending on unregister. If this
> - * happens, we simply run the queue
> - * unscheduled, resulting in a noop
> - * for this device.
> - */
> - linkwatch_run_queue();
> - break;
> - }
> -
> __rtnl_unlock();
>
> rebroadcast_time = jiffies;
> diff --git a/net/core/link_watch.c b/net/core/link_watch.c
> index 95098d1a49bd..9a0ea7cd68e4 100644
> --- a/net/core/link_watch.c
> +++ b/net/core/link_watch.c
> @@ -107,7 +107,8 @@ static void linkwatch_add_event(struct net_device *dev)
> unsigned long flags;
>
> spin_lock_irqsave(&lweventlist_lock, flags);
> - if (list_empty(&dev->link_watch_list)) {
> + if (list_empty(&dev->link_watch_list) &&
> + dev->reg_state < NETREG_UNREGISTERED) {
> list_add_tail(&dev->link_watch_list, &lweventlist);
> dev_hold_track(dev, &dev->linkwatch_dev_tracker, GFP_ATOMIC);
>
What about testing dev->reg_state in linkwatch_fire_event() before
setting the __LINK_STATE_LINKWATCH_PENDING bit, so that we don't leave
the device in an unexpected state?
Other than that, it looks good to me, but potentially quite risky.
Looking at the original report it looks like the issue could be
resolved with a more usb-specific change: e.g. it looks like
usbnet_defer_kevent() is not acquiring a dev reference as it should.
Have you considered that path?
Thanks,
Paolo
^ permalink raw reply
* Re: [PATCH] net/smc: sync err info when TCP connection is refused
From: Paolo Abeni @ 2022-04-21 8:09 UTC (permalink / raw)
To: yacanliu, kgraul, davem, kuba; +Cc: linux-s390, netdev, linux-kernel, liuyacan
In-Reply-To: <20220417123307.1094747-1-yacanliu@163.com>
On Sun, 2022-04-17 at 20:33 +0800, yacanliu@163.com wrote:
> From: liuyacan <liuyacan@corp.netease.com>
>
> In the current implementation, when TCP initiates a connection
> to an unavailable [ip,port], ECONNREFUSED will be stored in the
> TCP socket, but SMC will not. However, some apps (like curl) use
> getsockopt(,,SO_ERROR,,) to get the error information, which makes
> them miss the error message and behave strangely.
>
> Signed-off-by: liuyacan <liuyacan@corp.netease.com>
Could you please formally re-submit for -net (inclusing NET into the
patch subj) with a suitable 'fixes' tag? You can retain the already
collected reviewed/ack-by tags.
Thanks!
Paolo
^ permalink raw reply
* Re: [PATCH] openvswitch: meter: Remove unnecessary int
From: Paolo Abeni @ 2022-04-21 8:47 UTC (permalink / raw)
To: Solomon Tan, pshelar, davem, kuba, netdev, dev, linux-kernel
In-Reply-To: <Yly1t/mE6QAGPS0e@ArchDesktop>
On Mon, 2022-04-18 at 00:50 +0000, Solomon Tan wrote:
> This patch addresses the checkpatch.pl warning that long long is
> preferred over long long int.
Please don't do that. This kind of changes cause e.g. backporting issue
for any later relevant bugfix touching the same area, for no real
benefit.
Documentation/process/2.Process.rst
explicltly states to avoid this kind of patches.
>
> Signed-off-by: Solomon Tan <solomonbstoner@protonmail.ch>
> ---
> net/openvswitch/meter.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/net/openvswitch/meter.c b/net/openvswitch/meter.c
> index 04a060ac7fdf..a790920c11d6 100644
> --- a/net/openvswitch/meter.c
> +++ b/net/openvswitch/meter.c
> @@ -592,8 +592,8 @@ static int ovs_meter_cmd_del(struct sk_buff *skb, struct genl_info *info)
> bool ovs_meter_execute(struct datapath *dp, struct sk_buff *skb,
> struct sw_flow_key *key, u32 meter_id)
> {
> - long long int now_ms = div_u64(ktime_get_ns(), 1000 * 1000);
> - long long int long_delta_ms;
> + long long now_ms = div_u64(ktime_get_ns(), 1000 * 1000);
> + long long long_delta_ms;
> struct dp_meter_band *band;
> struct dp_meter *meter;
> int i, band_exceeded_max = -1;
Additionally the patch is mangled by non plain-text encoding.
For any later submissions (regarding some other different topic) please
ensure that your client/mailer send purely plain-text messages,
Thanks,
Paolo
^ 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