* [net-next 9/9] net/mlx5: Support ndo bridge_setlink and getlink
From: Saeed Mahameed @ 2019-02-22 21:44 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Huy Nguyen, Saeed Mahameed
In-Reply-To: <20190222214426.27833-1-saeedm@mellanox.com>
From: Huy Nguyen <huyn@mellanox.com>
Allow enabling VEPA mode on the HCA's port in legacy devlink mode.
Example:
bridge link set dev ens1f0 hwmode vepa
will turn on VEPA mode on the netdev ens1f0.
Signed-off-by: Huy Nguyen <huyn@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
.../net/ethernet/mellanox/mlx5/core/en_main.c | 59 +++++++++++++++++++
1 file changed, 59 insertions(+)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index 4f971f3d8ce7..e5f74eb986b3 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -35,6 +35,7 @@
#include <linux/mlx5/fs.h>
#include <net/vxlan.h>
#include <linux/bpf.h>
+#include <linux/if_bridge.h>
#include <net/page_pool.h>
#include "eswitch.h"
#include "en.h"
@@ -4305,6 +4306,61 @@ static int mlx5e_xdp(struct net_device *dev, struct netdev_bpf *xdp)
}
}
+#ifdef CONFIG_MLX5_ESWITCH
+static int mlx5e_bridge_getlink(struct sk_buff *skb, u32 pid, u32 seq,
+ struct net_device *dev, u32 filter_mask,
+ int nlflags)
+{
+ struct mlx5e_priv *priv = netdev_priv(dev);
+ struct mlx5_core_dev *mdev = priv->mdev;
+ u8 mode, setting;
+ int err;
+
+ err = mlx5_eswitch_get_vepa(mdev->priv.eswitch, &setting);
+ if (err)
+ return err;
+ mode = setting ? BRIDGE_MODE_VEPA : BRIDGE_MODE_VEB;
+ return ndo_dflt_bridge_getlink(skb, pid, seq, dev,
+ mode,
+ 0, 0, nlflags, filter_mask, NULL);
+}
+
+static int mlx5e_bridge_setlink(struct net_device *dev, struct nlmsghdr *nlh,
+ u16 flags, struct netlink_ext_ack *extack)
+{
+ struct mlx5e_priv *priv = netdev_priv(dev);
+ struct mlx5_core_dev *mdev = priv->mdev;
+ struct nlattr *attr, *br_spec;
+ u16 mode = BRIDGE_MODE_UNDEF;
+ u8 setting;
+ int rem;
+
+ br_spec = nlmsg_find_attr(nlh, sizeof(struct ifinfomsg), IFLA_AF_SPEC);
+ if (!br_spec)
+ return -EINVAL;
+
+ nla_for_each_nested(attr, br_spec, rem) {
+ if (nla_type(attr) != IFLA_BRIDGE_MODE)
+ continue;
+
+ if (nla_len(attr) < sizeof(mode))
+ return -EINVAL;
+
+ mode = nla_get_u16(attr);
+ if (mode > BRIDGE_MODE_VEPA)
+ return -EINVAL;
+
+ break;
+ }
+
+ if (mode == BRIDGE_MODE_UNDEF)
+ return -EINVAL;
+
+ setting = (mode == BRIDGE_MODE_VEPA) ? 1 : 0;
+ return mlx5_eswitch_set_vepa(mdev->priv.eswitch, setting);
+}
+#endif
+
const struct net_device_ops mlx5e_netdev_ops = {
.ndo_open = mlx5e_open,
.ndo_stop = mlx5e_close,
@@ -4331,6 +4387,9 @@ const struct net_device_ops mlx5e_netdev_ops = {
.ndo_rx_flow_steer = mlx5e_rx_flow_steer,
#endif
#ifdef CONFIG_MLX5_ESWITCH
+ .ndo_bridge_setlink = mlx5e_bridge_setlink,
+ .ndo_bridge_getlink = mlx5e_bridge_getlink,
+
/* SRIOV E-Switch NDOs */
.ndo_set_vf_mac = mlx5e_set_vf_mac,
.ndo_set_vf_vlan = mlx5e_set_vf_vlan,
--
2.20.1
^ permalink raw reply related
* [PATCH net-next] net: phy: check PMAPMD link status only in genphy_c45_read_link
From: Heiner Kallweit @ 2019-02-22 21:59 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev@vger.kernel.org
The current code reports a link as up if all devices (except a few
blacklisted ones) report the link as up. This breaks Aquantia AQCS109
for lower speeds because on this PHY the PCS link status reflects a
10G link only. For Marvell there's a similar issue, therefore PHYXS
device isn't checked.
There may be more PHYs where depending on the mode the link status
of only selected devices is relevant.
For now it seems to be sufficient to check the link status of the
PMAPMD device only. Leave the loop in the code to be prepared in
case we have to add functionality to check more than one device,
depending on the mode.
Successfully tested on a board with an AQCS109.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/phy/phy-c45.c | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
index d80797e8d..ce5fa5346 100644
--- a/drivers/net/phy/phy-c45.c
+++ b/drivers/net/phy/phy-c45.c
@@ -215,17 +215,10 @@ EXPORT_SYMBOL_GPL(genphy_c45_aneg_done);
*/
int genphy_c45_read_link(struct phy_device *phydev)
{
- u32 mmd_mask = phydev->c45_ids.devices_in_package;
+ u32 mmd_mask = MDIO_DEVS_PMAPMD;
int val, devad;
bool link = true;
- /* The vendor devads and C22EXT do not report link status. Avoid the
- * PHYXS instance as its status may depend on the MAC being
- * appropriately configured for the negotiated speed.
- */
- mmd_mask &= ~(MDIO_DEVS_VEND1 | MDIO_DEVS_VEND2 | MDIO_DEVS_C22EXT |
- MDIO_DEVS_PHYXS);
-
while (mmd_mask && link) {
devad = __ffs(mmd_mask);
mmd_mask &= ~BIT(devad);
--
2.20.1
^ permalink raw reply related
* [PATCH net] staging: fsl-dpaa2: ethsw: Add missing netdevice check
From: Florian Fainelli @ 2019-02-22 22:02 UTC (permalink / raw)
To: netdev
Cc: Florian Fainelli, Ioana Radulescu, Ioana Ciornei,
Greg Kroah-Hartman, open list:DPAA2 ETHERNET SWITCH DRIVER,
open list:STAGING SUBSYSTEM
port_switchdev_event() does not check that the target network device is
actually backed by the ethsw driver, this could be problematic in a
stacked environment case.
Fixes: 44baaa43d7cc ("staging: fsl-dpaa2/ethsw: Add Freescale DPAA2 Ethernet Switch driver")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/staging/fsl-dpaa2/ethsw/ethsw.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
index daabaceeea52..2edd82f5229a 100644
--- a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
+++ b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
@@ -1047,6 +1047,9 @@ static int port_switchdev_event(struct notifier_block *unused,
struct ethsw_switchdev_event_work *switchdev_work;
struct switchdev_notifier_fdb_info *fdb_info = ptr;
+ if (!ethsw_port_dev_check(dev))
+ return NOTIFY_DONE;
+
switchdev_work = kzalloc(sizeof(*switchdev_work), GFP_ATOMIC);
if (!switchdev_work)
return NOTIFY_BAD;
--
2.17.1
^ permalink raw reply related
* Re: [PATCH net 1/2] selftests: pmtu: disable dad in all namespaces
From: David Ahern @ 2019-02-22 22:05 UTC (permalink / raw)
To: Paolo Abeni, netdev; +Cc: David S. Miller, Stefano Brivio
In-Reply-To: <1d298b6839828c96cd8a0f50bdfac893cc645895.1550851038.git.pabeni@redhat.com>
On 2/22/19 11:06 AM, Paolo Abeni wrote:
> Otherwise, the configured IPv6 address could be still "tentative"
> at test time, possibly causing tests failures.
> We can also drop some sleep along the code and decrease the
> timeout for most commands so that the test runtime decreases.
>
> Fixes: d1f1b9cbf34c ("selftests: net: Introduce first PMTU test")
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> tools/testing/selftests/net/pmtu.sh | 28 +++++++++++++---------------
> 1 file changed, 13 insertions(+), 15 deletions(-)
>
Reviewed-by: David Ahern <dsahern@gmail.com>
^ permalink raw reply
* Re: [PATCH 1/2 v2] kprobe: Do not use uaccess functions to access kernel memory that can fault
From: Linus Torvalds @ 2019-02-22 21:59 UTC (permalink / raw)
To: David Miller
Cc: Alexei Starovoitov, Masami Hiramatsu, Steven Rostedt,
Andy Lutomirski, Linux List Kernel Mailing, Ingo Molnar,
Andrew Morton, stable, Changbin Du, Jann Horn, Kees Cook,
Andrew Lutomirski, Daniel Borkmann, Netdev, bpf
In-Reply-To: <20190222.133842.1637029078039923178.davem@davemloft.net>
On Fri, Feb 22, 2019 at 1:38 PM David Miller <davem@davemloft.net> wrote:
>
> Don't be surprised if we see more separation like this in the future too.
Yes, with the whole meltdown fiasco, there's actually more pressure to
add more support for separation of kernel/user address spaces. As Andy
pointed out, it's been discussed as a future wish-list for x86-64 too.
But yeah, right now the *common* architectures all distinguish kernel
and user space by pointers (ie x86-64, arm64 and powerpc).
Linus
^ permalink raw reply
* Re: [PATCH 1/2 v2] kprobe: Do not use uaccess functions to access kernel memory that can fault
From: Nadav Amit @ 2019-02-22 22:08 UTC (permalink / raw)
To: Jann Horn
Cc: Andy Lutomirski, Alexei Starovoitov, Steven Rostedt,
Linus Torvalds, Masami Hiramatsu, Linux List Kernel Mailing,
Ingo Molnar, Andrew Morton, Changbin Du, Kees Cook,
Andy Lutomirski, Daniel Borkmann, Network Development,
bpf@vger.kernel.org, Rick Edgecombe, Dave Hansen,
Peter Zijlstra (Intel)
In-Reply-To: <CAG48ez2tVwzhKPu1tvK+E52jUCq5vTwjssrtMaGuCO=kMpSXGw@mail.gmail.com>
> On Feb 22, 2019, at 1:43 PM, Jann Horn <jannh@google.com> wrote:
>
> (adding some people from the text_poke series to the thread, removing stable@)
>
> On Fri, Feb 22, 2019 at 8:55 PM Andy Lutomirski <luto@amacapital.net> wrote:
>>> On Feb 22, 2019, at 11:34 AM, Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
>>>> On Fri, Feb 22, 2019 at 02:30:26PM -0500, Steven Rostedt wrote:
>>>> On Fri, 22 Feb 2019 11:27:05 -0800
>>>> Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
>>>>
>>>>>> On Fri, Feb 22, 2019 at 09:43:14AM -0800, Linus Torvalds wrote:
>>>>>>
>>>>>> Then we should still probably fix up "__probe_kernel_read()" to not
>>>>>> allow user accesses. The easiest way to do that is actually likely to
>>>>>> use the "unsafe_get_user()" functions *without* doing a
>>>>>> uaccess_begin(), which will mean that modern CPU's will simply fault
>>>>>> on a kernel access to user space.
>>>>>
>>>>> On bpf side the bpf_probe_read() helper just calls probe_kernel_read()
>>>>> and users pass both user and kernel addresses into it and expect
>>>>> that the helper will actually try to read from that address.
>>>>>
>>>>> If __probe_kernel_read will suddenly start failing on all user addresses
>>>>> it will break the expectations.
>>>>> How do we solve it in bpf_probe_read?
>>>>> Call probe_kernel_read and if that fails call unsafe_get_user byte-by-byte
>>>>> in the loop?
>>>>> That's doable, but people already complain that bpf_probe_read() is slow
>>>>> and shows up in their perf report.
>>>>
>>>> We're changing kprobes to add a specific flag to say that we want to
>>>> differentiate between kernel or user reads. Can this be done with
>>>> bpf_probe_read()? If it's showing up in perf report, I doubt a single
>>>
>>> so you're saying you will break existing kprobe scripts?
>>> I don't think it's a good idea.
>>> It's not acceptable to break bpf_probe_read uapi.
>>
>> If so, the uapi is wrong: a long-sized number does not reliably identify an address if you don’t separately know whether it’s a user or kernel address. s390x and 4G:4G x86_32 are the notable exceptions. I have lobbied for RISC-V and future x86_64 to join the crowd. I don’t know whether I’ll win this fight, but the uapi will probably have to change for at least s390x.
>>
>> What to do about existing scripts is a different question.
>
> This lack of logical separation between user and kernel addresses
> might interact interestingly with the text_poke series, specifically
> "[PATCH v3 05/20] x86/alternative: Initialize temporary mm for
> patching" (https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flkml%2F20190221234451.17632-6-rick.p.edgecombe%40intel.com%2F&data=02%7C01%7Cnamit%40vmware.com%7Cd44d6f0765dd49b20db708d6990ee7e8%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636864686717142892&sdata=gVALdkEULEhj4iJNEWAGxyYWe2lxnHRdamW5ZA2A5RQ%3D&reserved=0)
> and "[PATCH v3 06/20] x86/alternative: Use temporary mm for text
> poking" (https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flkml%2F20190221234451.17632-7-rick.p.edgecombe%40intel.com%2F&data=02%7C01%7Cnamit%40vmware.com%7Cd44d6f0765dd49b20db708d6990ee7e8%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636864686717142892&sdata=nu2J1FtJsZJmt53SKJz8C8ktWE9eycwdAA%2BiCi1TfCc%3D&reserved=0),
> right? If someone manages to get a tracing BPF program to trigger in a
> task that has switched to the patching mm, could they use
> bpf_probe_write_user() - which uses probe_kernel_write() after
> checking that KERNEL_DS isn't active and that access_ok() passes - to
> overwrite kernel text that is mapped writable in the patching mm?
Yes, this is a good point. I guess text_poke() should be defined with
“__kprobes” and open-code memcpy.
Does it sound reasonable?
^ permalink raw reply
* [PATCH net-next v3 0/6] devlink: make ethtool compat reliable
From: Jakub Kicinski @ 2019-02-22 22:07 UTC (permalink / raw)
To: davem, jiri
Cc: mkubecek, andrew, f.fainelli, netdev, oss-drivers, Jakub Kicinski
Hi!
This is a follow up to the series which added device flash
updates via devlink. I went with the approach of adding a
new NDO in the end. It seems to end up looking cleaner
(the port favour patches are coming regardless ;)).
First patch removes the option to build devlink as a module.
Users can still decide to not build it, but the module option
ends up not being worth the maintenance cost.
Second patch fixes a potential issue with ethtool code, which
will become even more problematic as the NDO introduced by
the third patch does not hold RTNL. The new NDO allows the core
to get the devlink instance based on a netdev pointer.
Next the NDO is implemented in the NFP, and ethtool flashing
ops removed.
Last but not least missing checks for devlink->ops are added.
There is currently no driver registering devlink without ops,
so can just fix this in -next.
v3 (Florian):
- add missing checks for devlink->ops;
- move locking/holding into devlink_compat_ functions.
v2 (Michal): add netdev_to_devlink() in patch 3.
Jakub Kicinski (6):
net: devlink: turn devlink into a built-in
devlink: create a special NDO for getting the devlink instance
nfp: add .ndo_get_devlink
nfp: remove ethtool flashing fallback
devlink: hold a reference to the netdevice around ethtool compat
devlink: add missing NULL checks for devlink ops
drivers/infiniband/hw/bnxt_re/Kconfig | 1 -
drivers/infiniband/hw/mlx4/Kconfig | 1 -
drivers/net/Kconfig | 1 -
drivers/net/ethernet/broadcom/Kconfig | 1 -
drivers/net/ethernet/cavium/Kconfig | 1 -
drivers/net/ethernet/mellanox/mlx4/Kconfig | 1 -
.../net/ethernet/mellanox/mlx5/core/Kconfig | 1 -
drivers/net/ethernet/mellanox/mlxsw/Kconfig | 1 -
drivers/net/ethernet/netronome/Kconfig | 1 -
drivers/net/ethernet/netronome/nfp/nfp_app.h | 2 +
.../net/ethernet/netronome/nfp/nfp_devlink.c | 11 +++
.../ethernet/netronome/nfp/nfp_net_common.c | 1 +
.../ethernet/netronome/nfp/nfp_net_ethtool.c | 24 -----
.../net/ethernet/netronome/nfp/nfp_net_repr.c | 1 +
include/linux/netdevice.h | 6 ++
include/net/devlink.h | 19 ++--
net/Kconfig | 11 +--
net/core/devlink.c | 87 +++++++------------
net/core/ethtool.c | 13 +--
net/dsa/Kconfig | 2 +-
20 files changed, 69 insertions(+), 117 deletions(-)
--
2.19.2
^ permalink raw reply
* [PATCH net-next v3 1/6] net: devlink: turn devlink into a built-in
From: Jakub Kicinski @ 2019-02-22 22:07 UTC (permalink / raw)
To: davem, jiri
Cc: mkubecek, andrew, f.fainelli, netdev, oss-drivers, Jakub Kicinski
In-Reply-To: <20190222220758.7117-1-jakub.kicinski@netronome.com>
Being able to build devlink as a module causes growing pains.
First all drivers had to add a meta dependency to make sure
they are not built in when devlink is built as a module. Now
we are struggling to invoke ethtool compat code reliably.
Make devlink code built-in, users can still not build it at
all but the dynamically loadable module option is removed.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/infiniband/hw/bnxt_re/Kconfig | 1 -
drivers/infiniband/hw/mlx4/Kconfig | 1 -
drivers/net/Kconfig | 1 -
drivers/net/ethernet/broadcom/Kconfig | 1 -
drivers/net/ethernet/cavium/Kconfig | 1 -
drivers/net/ethernet/mellanox/mlx4/Kconfig | 1 -
drivers/net/ethernet/mellanox/mlx5/core/Kconfig | 1 -
drivers/net/ethernet/mellanox/mlxsw/Kconfig | 1 -
drivers/net/ethernet/netronome/Kconfig | 1 -
include/net/devlink.h | 10 ++++------
net/Kconfig | 11 +----------
net/core/devlink.c | 15 ++-------------
net/dsa/Kconfig | 2 +-
13 files changed, 8 insertions(+), 39 deletions(-)
diff --git a/drivers/infiniband/hw/bnxt_re/Kconfig b/drivers/infiniband/hw/bnxt_re/Kconfig
index 18f5ed082f41..19982a4a9bba 100644
--- a/drivers/infiniband/hw/bnxt_re/Kconfig
+++ b/drivers/infiniband/hw/bnxt_re/Kconfig
@@ -1,7 +1,6 @@
config INFINIBAND_BNXT_RE
tristate "Broadcom Netxtreme HCA support"
depends on ETHERNET && NETDEVICES && PCI && INET && DCB
- depends on MAY_USE_DEVLINK
select NET_VENDOR_BROADCOM
select BNXT
---help---
diff --git a/drivers/infiniband/hw/mlx4/Kconfig b/drivers/infiniband/hw/mlx4/Kconfig
index d1de3285fd88..4e9936731867 100644
--- a/drivers/infiniband/hw/mlx4/Kconfig
+++ b/drivers/infiniband/hw/mlx4/Kconfig
@@ -2,7 +2,6 @@ config MLX4_INFINIBAND
tristate "Mellanox ConnectX HCA support"
depends on NETDEVICES && ETHERNET && PCI && INET
depends on INFINIBAND_USER_ACCESS || !INFINIBAND_USER_ACCESS
- depends on MAY_USE_DEVLINK
select NET_VENDOR_MELLANOX
select MLX4_CORE
---help---
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 6210757772ed..5e4ca082cfcd 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -505,7 +505,6 @@ source "drivers/net/hyperv/Kconfig"
config NETDEVSIM
tristate "Simulated networking device"
depends on DEBUG_FS
- depends on MAY_USE_DEVLINK
help
This driver is a developer testing tool and software model that can
be used to test various control path networking APIs, especially
diff --git a/drivers/net/ethernet/broadcom/Kconfig b/drivers/net/ethernet/broadcom/Kconfig
index c1d3ee9baf7e..716bfbba59cf 100644
--- a/drivers/net/ethernet/broadcom/Kconfig
+++ b/drivers/net/ethernet/broadcom/Kconfig
@@ -194,7 +194,6 @@ config SYSTEMPORT
config BNXT
tristate "Broadcom NetXtreme-C/E support"
depends on PCI
- depends on MAY_USE_DEVLINK
select FW_LOADER
select LIBCRC32C
---help---
diff --git a/drivers/net/ethernet/cavium/Kconfig b/drivers/net/ethernet/cavium/Kconfig
index 05f4a3b21e29..6650e2a5f171 100644
--- a/drivers/net/ethernet/cavium/Kconfig
+++ b/drivers/net/ethernet/cavium/Kconfig
@@ -64,7 +64,6 @@ config CAVIUM_PTP
config LIQUIDIO
tristate "Cavium LiquidIO support"
depends on 64BIT && PCI
- depends on MAY_USE_DEVLINK
depends on PCI
imply PTP_1588_CLOCK
select FW_LOADER
diff --git a/drivers/net/ethernet/mellanox/mlx4/Kconfig b/drivers/net/ethernet/mellanox/mlx4/Kconfig
index f200b8c420d5..ff8057ed97ee 100644
--- a/drivers/net/ethernet/mellanox/mlx4/Kconfig
+++ b/drivers/net/ethernet/mellanox/mlx4/Kconfig
@@ -4,7 +4,6 @@
config MLX4_EN
tristate "Mellanox Technologies 1/10/40Gbit Ethernet support"
- depends on MAY_USE_DEVLINK
depends on PCI && NETDEVICES && ETHERNET && INET
select MLX4_CORE
imply PTP_1588_CLOCK
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/Kconfig b/drivers/net/ethernet/mellanox/mlx5/core/Kconfig
index 37a551436e4a..6debffb8336b 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/Kconfig
+++ b/drivers/net/ethernet/mellanox/mlx5/core/Kconfig
@@ -4,7 +4,6 @@
config MLX5_CORE
tristate "Mellanox 5th generation network adapters (ConnectX series) core driver"
- depends on MAY_USE_DEVLINK
depends on PCI
imply PTP_1588_CLOCK
imply VXLAN
diff --git a/drivers/net/ethernet/mellanox/mlxsw/Kconfig b/drivers/net/ethernet/mellanox/mlxsw/Kconfig
index b9a25aed5d11..9c195dfed031 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/Kconfig
+++ b/drivers/net/ethernet/mellanox/mlxsw/Kconfig
@@ -4,7 +4,6 @@
config MLXSW_CORE
tristate "Mellanox Technologies Switch ASICs support"
- depends on MAY_USE_DEVLINK
---help---
This driver supports Mellanox Technologies Switch ASICs family.
diff --git a/drivers/net/ethernet/netronome/Kconfig b/drivers/net/ethernet/netronome/Kconfig
index 66f15b05b65e..549898d5d450 100644
--- a/drivers/net/ethernet/netronome/Kconfig
+++ b/drivers/net/ethernet/netronome/Kconfig
@@ -19,7 +19,6 @@ config NFP
tristate "Netronome(R) NFP4000/NFP6000 NIC driver"
depends on PCI && PCI_MSI
depends on VXLAN || VXLAN=n
- depends on MAY_USE_DEVLINK
---help---
This driver supports the Netronome(R) NFP4000/NFP6000 based
cards working as a advanced Ethernet NIC. It works with both
diff --git a/include/net/devlink.h b/include/net/devlink.h
index a2da49dd9147..f9f7fe974652 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -707,6 +707,10 @@ devlink_health_reporter_priv(struct devlink_health_reporter *reporter);
int devlink_health_report(struct devlink_health_reporter *reporter,
const char *msg, void *priv_ctx);
+void devlink_compat_running_version(struct net_device *dev,
+ char *buf, size_t len);
+int devlink_compat_flash_update(struct net_device *dev, const char *file_name);
+
#else
static inline struct devlink *devlink_alloc(const struct devlink_ops *ops,
@@ -1190,13 +1194,7 @@ devlink_health_report(struct devlink_health_reporter *reporter,
{
return 0;
}
-#endif
-#if IS_REACHABLE(CONFIG_NET_DEVLINK)
-void devlink_compat_running_version(struct net_device *dev,
- char *buf, size_t len);
-int devlink_compat_flash_update(struct net_device *dev, const char *file_name);
-#else
static inline void
devlink_compat_running_version(struct net_device *dev, char *buf, size_t len)
{
diff --git a/net/Kconfig b/net/Kconfig
index 62da6148e9f8..1efe1f9ee492 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -429,21 +429,12 @@ config NET_SOCK_MSG
with the help of BPF programs.
config NET_DEVLINK
- tristate "Network physical/parent device Netlink interface"
+ bool "Network physical/parent device Netlink interface"
help
Network physical/parent device Netlink interface provides
infrastructure to support access to physical chip-wide config and
monitoring.
-config MAY_USE_DEVLINK
- tristate
- default m if NET_DEVLINK=m
- default y if NET_DEVLINK=y || NET_DEVLINK=n
- help
- Drivers using the devlink infrastructure should have a dependency
- on MAY_USE_DEVLINK to ensure they do not cause link errors when
- devlink is a loadable module and the driver using it is built-in.
-
config PAGE_POOL
bool
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 4f31ddc883e7..05e04ea0a5c7 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -6482,20 +6482,9 @@ int devlink_compat_flash_update(struct net_device *dev, const char *file_name)
return -EOPNOTSUPP;
}
-static int __init devlink_module_init(void)
+static int __init devlink_init(void)
{
return genl_register_family(&devlink_nl_family);
}
-static void __exit devlink_module_exit(void)
-{
- genl_unregister_family(&devlink_nl_family);
-}
-
-module_init(devlink_module_init);
-module_exit(devlink_module_exit);
-
-MODULE_LICENSE("GPL v2");
-MODULE_AUTHOR("Jiri Pirko <jiri@mellanox.com>");
-MODULE_DESCRIPTION("Network physical device Netlink interface");
-MODULE_ALIAS_GENL_FAMILY(DEVLINK_GENL_NAME);
+subsys_initcall(devlink_init);
diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig
index 91e52973ee13..fab49132345f 100644
--- a/net/dsa/Kconfig
+++ b/net/dsa/Kconfig
@@ -6,7 +6,7 @@ config HAVE_NET_DSA
config NET_DSA
tristate "Distributed Switch Architecture"
- depends on HAVE_NET_DSA && MAY_USE_DEVLINK
+ depends on HAVE_NET_DSA
depends on BRIDGE || BRIDGE=n
select NET_SWITCHDEV
select PHYLINK
--
2.19.2
^ permalink raw reply related
* [PATCH net-next v3 2/6] devlink: create a special NDO for getting the devlink instance
From: Jakub Kicinski @ 2019-02-22 22:07 UTC (permalink / raw)
To: davem, jiri
Cc: mkubecek, andrew, f.fainelli, netdev, oss-drivers, Jakub Kicinski
In-Reply-To: <20190222220758.7117-1-jakub.kicinski@netronome.com>
Instead of iterating over all devlink ports add a NDO which
will return the devlink instance from the driver.
v2: add the netdev_to_devlink() helper (Michal)
v3: check that devlink has ops (Florian)
Suggested-by: Jiri Pirko <jiri@resnulli.us>
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
include/linux/netdevice.h | 6 ++++
include/net/devlink.h | 9 ++++++
net/core/devlink.c | 58 ++++++++++-----------------------------
3 files changed, 30 insertions(+), 43 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index aab4d9f6613d..eebcef6b8191 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -940,6 +940,8 @@ struct dev_ifalias {
char ifalias[];
};
+struct devlink;
+
/*
* This structure defines the management hooks for network devices.
* The following hooks can be defined; unless noted otherwise, they are
@@ -1248,6 +1250,9 @@ struct dev_ifalias {
* that got dropped are freed/returned via xdp_return_frame().
* Returns negative number, means general error invoking ndo, meaning
* no frames were xmit'ed and core-caller will free all frames.
+ * struct devlink *(*ndo_get_devlink)(struct net_device *dev);
+ * Get devlink instance associated with a given netdev.
+ * Called with a reference on the device only, rtnl_lock is not held.
*/
struct net_device_ops {
int (*ndo_init)(struct net_device *dev);
@@ -1446,6 +1451,7 @@ struct net_device_ops {
u32 flags);
int (*ndo_xsk_async_xmit)(struct net_device *dev,
u32 queue_id);
+ struct devlink * (*ndo_get_devlink)(struct net_device *dev);
};
/**
diff --git a/include/net/devlink.h b/include/net/devlink.h
index f9f7fe974652..7f5a0bdca228 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -538,6 +538,15 @@ static inline struct devlink *priv_to_devlink(void *priv)
return container_of(priv, struct devlink, priv);
}
+static inline struct devlink *netdev_to_devlink(struct net_device *dev)
+{
+#if IS_ENABLED(CONFIG_NET_DEVLINK)
+ if (dev->netdev_ops->ndo_get_devlink)
+ return dev->netdev_ops->ndo_get_devlink(dev);
+#endif
+ return NULL;
+}
+
struct ib_device;
#if IS_ENABLED(CONFIG_NET_DEVLINK)
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 05e04ea0a5c7..a13055160be0 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -6397,9 +6397,6 @@ static void __devlink_compat_running_version(struct devlink *devlink,
struct sk_buff *msg;
int rem, err;
- if (!devlink->ops->info_get)
- return;
-
msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
if (!msg)
return;
@@ -6431,55 +6428,30 @@ static void __devlink_compat_running_version(struct devlink *devlink,
void devlink_compat_running_version(struct net_device *dev,
char *buf, size_t len)
{
- struct devlink_port *devlink_port;
struct devlink *devlink;
- mutex_lock(&devlink_mutex);
- list_for_each_entry(devlink, &devlink_list, list) {
- mutex_lock(&devlink->lock);
- list_for_each_entry(devlink_port, &devlink->port_list, list) {
- if (devlink_port->type == DEVLINK_PORT_TYPE_ETH &&
- devlink_port->type_dev == dev) {
- __devlink_compat_running_version(devlink,
- buf, len);
- mutex_unlock(&devlink->lock);
- goto out;
- }
- }
- mutex_unlock(&devlink->lock);
- }
-out:
- mutex_unlock(&devlink_mutex);
+ devlink = netdev_to_devlink(dev);
+ if (!devlink || !devlink->ops || !devlink->ops->info_get)
+ return;
+
+ mutex_lock(&devlink->lock);
+ __devlink_compat_running_version(devlink, buf, len);
+ mutex_unlock(&devlink->lock);
}
int devlink_compat_flash_update(struct net_device *dev, const char *file_name)
{
- struct devlink_port *devlink_port;
struct devlink *devlink;
+ int ret;
- mutex_lock(&devlink_mutex);
- list_for_each_entry(devlink, &devlink_list, list) {
- mutex_lock(&devlink->lock);
- list_for_each_entry(devlink_port, &devlink->port_list, list) {
- int ret = -EOPNOTSUPP;
-
- if (devlink_port->type != DEVLINK_PORT_TYPE_ETH ||
- devlink_port->type_dev != dev)
- continue;
-
- mutex_unlock(&devlink_mutex);
- if (devlink->ops->flash_update)
- ret = devlink->ops->flash_update(devlink,
- file_name,
- NULL, NULL);
- mutex_unlock(&devlink->lock);
- return ret;
- }
- mutex_unlock(&devlink->lock);
- }
- mutex_unlock(&devlink_mutex);
+ devlink = netdev_to_devlink(dev);
+ if (!devlink || !devlink->ops || !devlink->ops->flash_update)
+ return -EOPNOTSUPP;
- return -EOPNOTSUPP;
+ mutex_lock(&devlink->lock);
+ ret = devlink->ops->flash_update(devlink, file_name, NULL, NULL);
+ mutex_unlock(&devlink->lock);
+ return ret;
}
static int __init devlink_init(void)
--
2.19.2
^ permalink raw reply related
* [PATCH net-next v3 3/6] nfp: add .ndo_get_devlink
From: Jakub Kicinski @ 2019-02-22 22:07 UTC (permalink / raw)
To: davem, jiri
Cc: mkubecek, andrew, f.fainelli, netdev, oss-drivers, Jakub Kicinski
In-Reply-To: <20190222220758.7117-1-jakub.kicinski@netronome.com>
Support getting devlink instance from a new NDO.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
drivers/net/ethernet/netronome/nfp/nfp_app.h | 2 ++
drivers/net/ethernet/netronome/nfp/nfp_devlink.c | 11 +++++++++++
drivers/net/ethernet/netronome/nfp/nfp_net_common.c | 1 +
drivers/net/ethernet/netronome/nfp/nfp_net_repr.c | 1 +
4 files changed, 15 insertions(+)
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_app.h b/drivers/net/ethernet/netronome/nfp/nfp_app.h
index d578d856a009..f8d422713705 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_app.h
+++ b/drivers/net/ethernet/netronome/nfp/nfp_app.h
@@ -433,4 +433,6 @@ int nfp_app_nic_vnic_alloc(struct nfp_app *app, struct nfp_net *nn,
int nfp_app_nic_vnic_init_phy_port(struct nfp_pf *pf, struct nfp_app *app,
struct nfp_net *nn, unsigned int id);
+struct devlink *nfp_devlink_get_devlink(struct net_device *netdev);
+
#endif
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
index db2da99f6aa7..e9eca99cf493 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
@@ -376,3 +376,14 @@ void nfp_devlink_port_unregister(struct nfp_port *port)
{
devlink_port_unregister(&port->dl_port);
}
+
+struct devlink *nfp_devlink_get_devlink(struct net_device *netdev)
+{
+ struct nfp_app *app;
+
+ app = nfp_app_from_netdev(netdev);
+ if (!app)
+ return NULL;
+
+ return priv_to_devlink(app->pf);
+}
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
index 776f6c07701b..6d1b8816552e 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
@@ -3531,6 +3531,7 @@ const struct net_device_ops nfp_net_netdev_ops = {
.ndo_udp_tunnel_del = nfp_net_del_vxlan_port,
.ndo_bpf = nfp_net_xdp,
.ndo_get_port_parent_id = nfp_port_get_port_parent_id,
+ .ndo_get_devlink = nfp_devlink_get_devlink,
};
/**
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_repr.c b/drivers/net/ethernet/netronome/nfp/nfp_net_repr.c
index 62839807e21e..d2c803bb4e56 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_repr.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_repr.c
@@ -273,6 +273,7 @@ const struct net_device_ops nfp_repr_netdev_ops = {
.ndo_set_features = nfp_port_set_features,
.ndo_set_mac_address = eth_mac_addr,
.ndo_get_port_parent_id = nfp_port_get_port_parent_id,
+ .ndo_get_devlink = nfp_devlink_get_devlink,
};
void
--
2.19.2
^ permalink raw reply related
* [PATCH net-next v3 5/6] devlink: hold a reference to the netdevice around ethtool compat
From: Jakub Kicinski @ 2019-02-22 22:07 UTC (permalink / raw)
To: davem, jiri
Cc: mkubecek, andrew, f.fainelli, netdev, oss-drivers, Jakub Kicinski
In-Reply-To: <20190222220758.7117-1-jakub.kicinski@netronome.com>
When ethtool is calling into devlink compat code make sure we have
a reference on the netdevice on which the operation was invoked.
v3: move the hold/lock logic into devlink_compat_* functions (Florian)
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
net/core/devlink.c | 34 +++++++++++++++++++++++-----------
net/core/ethtool.c | 13 ++-----------
2 files changed, 25 insertions(+), 22 deletions(-)
diff --git a/net/core/devlink.c b/net/core/devlink.c
index a13055160be0..78c6ea1870ca 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -6430,27 +6430,39 @@ void devlink_compat_running_version(struct net_device *dev,
{
struct devlink *devlink;
+ dev_hold(dev);
+ rtnl_unlock();
+
devlink = netdev_to_devlink(dev);
- if (!devlink || !devlink->ops || !devlink->ops->info_get)
- return;
+ if (devlink && devlink->ops && devlink->ops->info_get) {
+ mutex_lock(&devlink->lock);
+ __devlink_compat_running_version(devlink, buf, len);
+ mutex_unlock(&devlink->lock);
+ }
- mutex_lock(&devlink->lock);
- __devlink_compat_running_version(devlink, buf, len);
- mutex_unlock(&devlink->lock);
+ rtnl_lock();
+ dev_put(dev);
}
int devlink_compat_flash_update(struct net_device *dev, const char *file_name)
{
struct devlink *devlink;
- int ret;
+ int ret = -EOPNOTSUPP;
+
+ dev_hold(dev);
+ rtnl_unlock();
devlink = netdev_to_devlink(dev);
- if (!devlink || !devlink->ops || !devlink->ops->flash_update)
- return -EOPNOTSUPP;
+ if (devlink && devlink->ops && devlink->ops->flash_update) {
+ mutex_lock(&devlink->lock);
+ ret = devlink->ops->flash_update(devlink, file_name,
+ NULL, NULL);
+ mutex_unlock(&devlink->lock);
+ }
+
+ rtnl_lock();
+ dev_put(dev);
- mutex_lock(&devlink->lock);
- ret = devlink->ops->flash_update(devlink, file_name, NULL, NULL);
- mutex_unlock(&devlink->lock);
return ret;
}
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 1320e8dce559..47558ffae423 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -805,11 +805,9 @@ static noinline_for_stack int ethtool_get_drvinfo(struct net_device *dev,
if (ops->get_eeprom_len)
info.eedump_len = ops->get_eeprom_len(dev);
- rtnl_unlock();
if (!info.fw_version[0])
devlink_compat_running_version(dev, info.fw_version,
sizeof(info.fw_version));
- rtnl_lock();
if (copy_to_user(useraddr, &info, sizeof(info)))
return -EFAULT;
@@ -2040,15 +2038,8 @@ static noinline_for_stack int ethtool_flash_device(struct net_device *dev,
return -EFAULT;
efl.data[ETHTOOL_FLASH_MAX_FILENAME - 1] = 0;
- if (!dev->ethtool_ops->flash_device) {
- int ret;
-
- rtnl_unlock();
- ret = devlink_compat_flash_update(dev, efl.data);
- rtnl_lock();
-
- return ret;
- }
+ if (!dev->ethtool_ops->flash_device)
+ return devlink_compat_flash_update(dev, efl.data);
return dev->ethtool_ops->flash_device(dev, &efl);
}
--
2.19.2
^ permalink raw reply related
* [PATCH net-next v3 6/6] devlink: add missing NULL checks for devlink ops
From: Jakub Kicinski @ 2019-02-22 22:07 UTC (permalink / raw)
To: davem, jiri
Cc: mkubecek, andrew, f.fainelli, netdev, oss-drivers, Jakub Kicinski
In-Reply-To: <20190222220758.7117-1-jakub.kicinski@netronome.com>
Commit 76726ccb7f46 ("devlink: add flash update command") and
commit 2d8dc5bbf4e7 ("devlink: Add support for reload")
access devlink ops without NULL-checking. Add the missing checks.
Note that all drivers currently implementing devlink pass non-NULL
ops, so this is not a problem.
Reported-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
net/core/devlink.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 78c6ea1870ca..38cb0239dede 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -2651,7 +2651,7 @@ static int devlink_nl_cmd_reload(struct sk_buff *skb, struct genl_info *info)
struct devlink *devlink = info->user_ptr[0];
int err;
- if (!devlink->ops->reload)
+ if (!devlink->ops || !devlink->ops->reload)
return -EOPNOTSUPP;
err = devlink_resources_validate(devlink, NULL, info);
@@ -2669,7 +2669,7 @@ static int devlink_nl_cmd_flash_update(struct sk_buff *skb,
const char *file_name, *component;
struct nlattr *nla_component;
- if (!devlink->ops->flash_update)
+ if (!devlink->ops || !devlink->ops->flash_update)
return -EOPNOTSUPP;
if (!info->attrs[DEVLINK_ATTR_FLASH_UPDATE_FILE_NAME])
--
2.19.2
^ permalink raw reply related
* [PATCH net-next v3 4/6] nfp: remove ethtool flashing fallback
From: Jakub Kicinski @ 2019-02-22 22:07 UTC (permalink / raw)
To: davem, jiri
Cc: mkubecek, andrew, f.fainelli, netdev, oss-drivers, Jakub Kicinski
In-Reply-To: <20190222220758.7117-1-jakub.kicinski@netronome.com>
Now that devlink fallback will be called reliably, we can remove
the ethtool flashing code.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
.../ethernet/netronome/nfp/nfp_net_ethtool.c | 24 -------------------
1 file changed, 24 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c b/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
index 8f189149efc5..690b62718dbb 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
@@ -1234,28 +1234,6 @@ static int nfp_net_set_channels(struct net_device *netdev,
return nfp_net_set_num_rings(nn, total_rx, total_tx);
}
-static int
-nfp_net_flash_device(struct net_device *netdev, struct ethtool_flash *flash)
-{
- struct nfp_app *app;
- int ret;
-
- if (flash->region != ETHTOOL_FLASH_ALL_REGIONS)
- return -EOPNOTSUPP;
-
- app = nfp_app_from_netdev(netdev);
- if (!app)
- return -EOPNOTSUPP;
-
- dev_hold(netdev);
- rtnl_unlock();
- ret = nfp_flash_update_common(app->pf, flash->data, NULL);
- rtnl_lock();
- dev_put(netdev);
-
- return ret;
-}
-
static const struct ethtool_ops nfp_net_ethtool_ops = {
.get_drvinfo = nfp_net_get_drvinfo,
.get_link = ethtool_op_get_link,
@@ -1266,7 +1244,6 @@ static const struct ethtool_ops nfp_net_ethtool_ops = {
.get_sset_count = nfp_net_get_sset_count,
.get_rxnfc = nfp_net_get_rxnfc,
.set_rxnfc = nfp_net_set_rxnfc,
- .flash_device = nfp_net_flash_device,
.get_rxfh_indir_size = nfp_net_get_rxfh_indir_size,
.get_rxfh_key_size = nfp_net_get_rxfh_key_size,
.get_rxfh = nfp_net_get_rxfh,
@@ -1292,7 +1269,6 @@ const struct ethtool_ops nfp_port_ethtool_ops = {
.get_strings = nfp_port_get_strings,
.get_ethtool_stats = nfp_port_get_stats,
.get_sset_count = nfp_port_get_sset_count,
- .flash_device = nfp_net_flash_device,
.set_dump = nfp_app_set_dump,
.get_dump_flag = nfp_app_get_dump_flag,
.get_dump_data = nfp_app_get_dump_data,
--
2.19.2
^ permalink raw reply related
* Re: [PATCH net-next v2] ip_tunnel: Add dst_cache management lwtunnel_state of ip tunnel
From: David Ahern @ 2019-02-22 22:16 UTC (permalink / raw)
To: wenxu, davem, netdev
In-Reply-To: <8b96ffcb-fe29-1c49-a266-3e9d20b60d89@ucloud.cn>
On 2/22/19 10:21 AM, wenxu wrote:
> On 2019/2/22 下午11:03, David Ahern wrote:
>> On 2/21/19 11:14 PM, wenxu wrote:
>>> build_state in the rcu_read_lock and disable the preempt
>>>
>>> rcu_read_lock();
>>> ops = rcu_dereference(lwtun_encaps[encap_type]);
>>> if (likely(ops && ops->build_state && try_module_get(ops->owner))) {
>>> found = true;
>>> ret = ops->build_state(encap, family, cfg, lws, extack);
>>> if (ret)
>>> module_put(ops->owner);
>>> }
>>> rcu_read_unlock();
>>>
>> Missed that.
>>
>> Once a reference is taken the rcu_read_lock can be dropped before
>> calling build_state allowing the allocations to be GFP_KERNEL.
>>
> I don't think the rcu_read_lock can be dropped. The whole operation of the reference should be protect
>
> in the rcu_read_lock that ensure the ops will not be destroy (if it will be)
>
If 'try_module_get(ops->owner)' succeeds then a reference is held so the
ops will not go away.
^ permalink raw reply
* Re: [PATCH 1/2 v2] kprobe: Do not use uaccess functions to access kernel memory that can fault
From: Jann Horn @ 2019-02-22 22:17 UTC (permalink / raw)
To: Nadav Amit
Cc: Andy Lutomirski, Alexei Starovoitov, Steven Rostedt,
Linus Torvalds, Masami Hiramatsu, Linux List Kernel Mailing,
Ingo Molnar, Andrew Morton, Changbin Du, Kees Cook,
Andy Lutomirski, Daniel Borkmann, Network Development,
bpf@vger.kernel.org, Rick Edgecombe, Dave Hansen,
Peter Zijlstra (Intel)
In-Reply-To: <0ED6836E-3432-4E1C-BABC-BEA6BDD36287@vmware.com>
On Fri, Feb 22, 2019 at 11:08 PM Nadav Amit <namit@vmware.com> wrote:
> > On Feb 22, 2019, at 1:43 PM, Jann Horn <jannh@google.com> wrote:
> >
> > (adding some people from the text_poke series to the thread, removing stable@)
> >
> > On Fri, Feb 22, 2019 at 8:55 PM Andy Lutomirski <luto@amacapital.net> wrote:
> >>> On Feb 22, 2019, at 11:34 AM, Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> >>>> On Fri, Feb 22, 2019 at 02:30:26PM -0500, Steven Rostedt wrote:
> >>>> On Fri, 22 Feb 2019 11:27:05 -0800
> >>>> Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> >>>>
> >>>>>> On Fri, Feb 22, 2019 at 09:43:14AM -0800, Linus Torvalds wrote:
> >>>>>>
> >>>>>> Then we should still probably fix up "__probe_kernel_read()" to not
> >>>>>> allow user accesses. The easiest way to do that is actually likely to
> >>>>>> use the "unsafe_get_user()" functions *without* doing a
> >>>>>> uaccess_begin(), which will mean that modern CPU's will simply fault
> >>>>>> on a kernel access to user space.
> >>>>>
> >>>>> On bpf side the bpf_probe_read() helper just calls probe_kernel_read()
> >>>>> and users pass both user and kernel addresses into it and expect
> >>>>> that the helper will actually try to read from that address.
> >>>>>
> >>>>> If __probe_kernel_read will suddenly start failing on all user addresses
> >>>>> it will break the expectations.
> >>>>> How do we solve it in bpf_probe_read?
> >>>>> Call probe_kernel_read and if that fails call unsafe_get_user byte-by-byte
> >>>>> in the loop?
> >>>>> That's doable, but people already complain that bpf_probe_read() is slow
> >>>>> and shows up in their perf report.
> >>>>
> >>>> We're changing kprobes to add a specific flag to say that we want to
> >>>> differentiate between kernel or user reads. Can this be done with
> >>>> bpf_probe_read()? If it's showing up in perf report, I doubt a single
> >>>
> >>> so you're saying you will break existing kprobe scripts?
> >>> I don't think it's a good idea.
> >>> It's not acceptable to break bpf_probe_read uapi.
> >>
> >> If so, the uapi is wrong: a long-sized number does not reliably identify an address if you don’t separately know whether it’s a user or kernel address. s390x and 4G:4G x86_32 are the notable exceptions. I have lobbied for RISC-V and future x86_64 to join the crowd. I don’t know whether I’ll win this fight, but the uapi will probably have to change for at least s390x.
> >>
> >> What to do about existing scripts is a different question.
> >
> > This lack of logical separation between user and kernel addresses
> > might interact interestingly with the text_poke series, specifically
> > "[PATCH v3 05/20] x86/alternative: Initialize temporary mm for
> > patching" (https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flkml%2F20190221234451.17632-6-rick.p.edgecombe%40intel.com%2F&data=02%7C01%7Cnamit%40vmware.com%7Cd44d6f0765dd49b20db708d6990ee7e8%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636864686717142892&sdata=gVALdkEULEhj4iJNEWAGxyYWe2lxnHRdamW5ZA2A5RQ%3D&reserved=0)
> > and "[PATCH v3 06/20] x86/alternative: Use temporary mm for text
> > poking" (https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flkml%2F20190221234451.17632-7-rick.p.edgecombe%40intel.com%2F&data=02%7C01%7Cnamit%40vmware.com%7Cd44d6f0765dd49b20db708d6990ee7e8%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636864686717142892&sdata=nu2J1FtJsZJmt53SKJz8C8ktWE9eycwdAA%2BiCi1TfCc%3D&reserved=0),
> > right? If someone manages to get a tracing BPF program to trigger in a
> > task that has switched to the patching mm, could they use
> > bpf_probe_write_user() - which uses probe_kernel_write() after
> > checking that KERNEL_DS isn't active and that access_ok() passes - to
> > overwrite kernel text that is mapped writable in the patching mm?
>
> Yes, this is a good point. I guess text_poke() should be defined with
> “__kprobes” and open-code memcpy.
>
> Does it sound reasonable?
Doesn't __text_poke() as implemented in the proposed patch use a
couple other kernel functions, too? Like switch_mm_irqs_off() and
pte_clear() (which can be a call into a separate function on paravirt
kernels)?
^ permalink raw reply
* Re: [PATCH net 2/2] selftests: pmtu: add explicit tests for pmtu exceptions cleanup
From: Stefano Brivio @ 2019-02-22 22:22 UTC (permalink / raw)
To: Paolo Abeni; +Cc: netdev, David S. Miller, David Ahern
In-Reply-To: <06a88ed3b2efab312f6301e9fe8eabd4383a9d3a.1550851038.git.pabeni@redhat.com>
On Fri, 22 Feb 2019 17:06:33 +0100
Paolo Abeni <pabeni@redhat.com> wrote:
> @@ -135,7 +135,9 @@ tests="
> pmtu_vti6_default_mtu vti6: default MTU assignment
> pmtu_vti4_link_add_mtu vti4: MTU setting on link creation
> pmtu_vti6_link_add_mtu vti6: MTU setting on link creation
> - pmtu_vti6_link_change_mtu vti6: MTU changes on link changes"
> + pmtu_vti6_link_change_mtu vti6: MTU changes on link changes
> + pmtu_ipv4_exception_cleanup ipv4: cleanup of cached exceptions
> + pmtu_ipv6_exception_cleanup ipv6: cleanup of cached exceptions"
I forgot: while at it, could you please also add test descriptions above
as it was done for the other tests? Thank you.
--
Stefano
^ permalink raw reply
* Re: [PATCH 1/2 v2] kprobe: Do not use uaccess functions to access kernel memory that can fault
From: Nadav Amit @ 2019-02-22 22:21 UTC (permalink / raw)
To: Jann Horn
Cc: Andy Lutomirski, Alexei Starovoitov, Steven Rostedt,
Linus Torvalds, Masami Hiramatsu, Linux List Kernel Mailing,
Ingo Molnar, Andrew Morton, Changbin Du, Kees Cook,
Andy Lutomirski, Daniel Borkmann, Network Development,
bpf@vger.kernel.org, Rick Edgecombe, Dave Hansen,
Peter Zijlstra (Intel)
In-Reply-To: <CAG48ez10RH2YhQ0wnEDOpKoGjrociq+zK0En6Aq06Yg9W3o5aQ@mail.gmail.com>
> On Feb 22, 2019, at 2:17 PM, Jann Horn <jannh@google.com> wrote:
>
> On Fri, Feb 22, 2019 at 11:08 PM Nadav Amit <namit@vmware.com> wrote:
>>> On Feb 22, 2019, at 1:43 PM, Jann Horn <jannh@google.com> wrote:
>>>
>>> (adding some people from the text_poke series to the thread, removing stable@)
>>>
>>> On Fri, Feb 22, 2019 at 8:55 PM Andy Lutomirski <luto@amacapital.net> wrote:
>>>>> On Feb 22, 2019, at 11:34 AM, Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
>>>>>> On Fri, Feb 22, 2019 at 02:30:26PM -0500, Steven Rostedt wrote:
>>>>>> On Fri, 22 Feb 2019 11:27:05 -0800
>>>>>> Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
>>>>>>
>>>>>>>> On Fri, Feb 22, 2019 at 09:43:14AM -0800, Linus Torvalds wrote:
>>>>>>>>
>>>>>>>> Then we should still probably fix up "__probe_kernel_read()" to not
>>>>>>>> allow user accesses. The easiest way to do that is actually likely to
>>>>>>>> use the "unsafe_get_user()" functions *without* doing a
>>>>>>>> uaccess_begin(), which will mean that modern CPU's will simply fault
>>>>>>>> on a kernel access to user space.
>>>>>>>
>>>>>>> On bpf side the bpf_probe_read() helper just calls probe_kernel_read()
>>>>>>> and users pass both user and kernel addresses into it and expect
>>>>>>> that the helper will actually try to read from that address.
>>>>>>>
>>>>>>> If __probe_kernel_read will suddenly start failing on all user addresses
>>>>>>> it will break the expectations.
>>>>>>> How do we solve it in bpf_probe_read?
>>>>>>> Call probe_kernel_read and if that fails call unsafe_get_user byte-by-byte
>>>>>>> in the loop?
>>>>>>> That's doable, but people already complain that bpf_probe_read() is slow
>>>>>>> and shows up in their perf report.
>>>>>>
>>>>>> We're changing kprobes to add a specific flag to say that we want to
>>>>>> differentiate between kernel or user reads. Can this be done with
>>>>>> bpf_probe_read()? If it's showing up in perf report, I doubt a single
>>>>>
>>>>> so you're saying you will break existing kprobe scripts?
>>>>> I don't think it's a good idea.
>>>>> It's not acceptable to break bpf_probe_read uapi.
>>>>
>>>> If so, the uapi is wrong: a long-sized number does not reliably identify an address if you don’t separately know whether it’s a user or kernel address. s390x and 4G:4G x86_32 are the notable exceptions. I have lobbied for RISC-V and future x86_64 to join the crowd. I don’t know whether I’ll win this fight, but the uapi will probably have to change for at least s390x.
>>>>
>>>> What to do about existing scripts is a different question.
>>>
>>> This lack of logical separation between user and kernel addresses
>>> might interact interestingly with the text_poke series, specifically
>>> "[PATCH v3 05/20] x86/alternative: Initialize temporary mm for
>>> patching" (https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flkml%2F20190221234451.17632-6-rick.p.edgecombe%40intel.com%2F&data=02%7C01%7Cnamit%40vmware.com%7Cf2513009ef734ecd6b0d08d69913a5ae%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636864707020821793&sdata=HAbnDcrBne64JyPuVUMKmM7nQk67F%2BFvjuXEn8TmHeo%3D&reserved=0)
>>> and "[PATCH v3 06/20] x86/alternative: Use temporary mm for text
>>> poking" (https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flkml%2F20190221234451.17632-7-rick.p.edgecombe%40intel.com%2F&data=02%7C01%7Cnamit%40vmware.com%7Cf2513009ef734ecd6b0d08d69913a5ae%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636864707020821793&sdata=vNRIMKtFDy%2F3z5FlTwDiJY6VGEV%2FMHgQPTdFSFtCo4s%3D&reserved=0),
>>> right? If someone manages to get a tracing BPF program to trigger in a
>>> task that has switched to the patching mm, could they use
>>> bpf_probe_write_user() - which uses probe_kernel_write() after
>>> checking that KERNEL_DS isn't active and that access_ok() passes - to
>>> overwrite kernel text that is mapped writable in the patching mm?
>>
>> Yes, this is a good point. I guess text_poke() should be defined with
>> “__kprobes” and open-code memcpy.
>>
>> Does it sound reasonable?
>
> Doesn't __text_poke() as implemented in the proposed patch use a
> couple other kernel functions, too? Like switch_mm_irqs_off() and
> pte_clear() (which can be a call into a separate function on paravirt
> kernels)?
I will move the pte_clear() to be done after the poking mm was unloaded.
Give me a few minutes to send a sketch of what I think should be done.
^ permalink raw reply
* Re: Yet another approach for implementing connection tracking offload
From: Marcelo Leitner @ 2019-02-22 22:23 UTC (permalink / raw)
To: Yossi Kuperman
Cc: Guy Shattah, Aaron Conole, John Hurley, Simon Horman,
Justin Pettit, Gregory Rose, Eelco Chaudron, Flavio Leitner,
Florian Westphal, Jiri Pirko, Rashid Khan, Sushil Kulkarni,
Andy Gospodarek, Roi Dayan, Or Gerlitz, Rony Efraim,
davem@davemloft.net, Paul Blakey, netdev@vger.kernel.org
In-Reply-To: <65CD6CF6-C50F-47F7-85DB-12D480FA4712@mellanox.com>
On Mon, Feb 18, 2019 at 07:00:19PM +0000, Yossi Kuperman wrote:
> Hello All,
>
> Following is a description of yet another possible approach to
> implement connection tracking offload. We would like to hear your
> opinion. There is the “native” way of implementing such an offload
> by mirroring the software tables to hardware. This way seems
> straightforward and simple, but real life is much more complicated
> than that. Alternatively, we can merge the data-path flows
> (separated by recirc_id) and offload a single flow to hardware.
>
> The general idea is quite simple. When OVS-daemon configures TC with
> a filter that recirculate, the driver merely pretends to offload it
> and return success. Upon packet arrival (in software) we let it
This has potential to be a support nightmare: things that should and
seems to be but are actually not and expected not to be.
IOW, not_in_hw should still be there somehow.
> traverse TC as usual, except for now we notify the driver on each
> successful match. By doing this, the driver has all the necessary
> information to merge the participating flows---including connection
> tracking 5-tuple---into one equivalent flow. We do such a merge and
> offload only if the connection is established. Note: the same
> mechanism to communicate a 5-tuple to the driver can be used to
> notify on a filter match.
>
> It is the driver responsibility to build and maintain the list of
> filters a (specific) packet hit along the TC walk. Once we reach the
I'm assuming this could be a shared code amongst the drivers, like
DIM. We really don't want different algorithms for this.
With that, we would be using the driver as just a temporary storage,
for the matches/actions. Maybe we can do that with skb extensions?
Like, turn on a flight recorder extention and have one especial last
tc action (or even embed it into core tc) to process it when the
traversing finishes. And only then call the driver to update whatever
is needed.
> last filter (a terminating one, e.g., forward) the driver posts a
> work on a dedicated work-queue. In this work-queue context, we merge
> the participating filters and create a new filter that is logically
> equal (match + actions). The merge itself is not complicated as it
> might seems—TC does all the heavy lifting, this is not a random list
> of filters. At this point, we configure the hardware with one
> filter, either we have a match and the packet is handled by the
> hardware, or we don’t and the packet goes to software unmodified.
>
> Going along this path we must tackle two things: 1) counters and 2)
> TC filter deletion. 1) We must maintain TC counters as the user
> expect. Each merged filter holds a list of filters it is derived
> from, parents. Once an update is available for a merged filter
> counter, the driver must update the corresponding parents
> appropriately. 2) Upon TC filer deletion it is mandatory to remove
> all the derived (merged) filters from the hardware as consequence.
I'm failing to see how this would address two features of CT:
- window validation. How would the card know that it should perform
window validation on these packets?
- flow stats. If it offloads the result of a merge, how can it
retrieve the specific stats for each 5-tuple? Even with the lists
you mentioned, once the 5-tuples are aggregated, we can't separate
them anymore.
>
>
> Pros & Cons
>
> Pros: 1) Circumvent the complexity involved with continuation in
> software where the hardware left off. 2) Simplifies the hardware
> pipeline with only one filter and might improve the overall
> performance.
>
> Cons: 1) Only applicable to OVS-oriented filters, will not support
> priorities and overlapping filters. 2) Merger logic might consume
> CPU cycles which might impact the rate of filters we can offload.
> However, this overhead is believed to be negligible, if implemented
> carefully. 3) Requires TC/flower to notify the driver on each filter
> match (that is the only change needed above the driver).
On more general comments, I have the feeling that this would be best
done in OvS:
- it has 2 flow "translations": one from OpenFlow to OvS/tc sw
datapath, and then OvS/tc sw to hw. If the resulting flows are
better for the hw, aren't them better for sw datapath too?
- if this is for OvS-oriented filters, why can't/shouldn't OvS do this
processing instead?
Somehow I keep thinking this is replacing conntrack (as in the
conntrack subsystem) with ipset offloading.
All in all, any approach is probably fine but I'm struggling to see a
path forward from this to actual CT offloading. Maybe I missed
something.
Cheers,
Marcelo
>
>
> Both approaches share the same software model, most of the code
> above the driver is shared. This approach can be considered
> temporary until the hardware will mature.
>
> What do you think about this approach?
>
> If something is not clear please let me know and I will do my best
> to clarify.
>
> Cheers, Kuperman
>
^ permalink raw reply
* Re: [PATCH][net-next] ipv6: sanitize RCU usage on fib6_next
From: David Ahern @ 2019-02-22 22:35 UTC (permalink / raw)
To: Li RongQing, netdev
In-Reply-To: <1550828682-10608-1-git-send-email-lirongqing@baidu.com>
On 2/22/19 4:44 AM, Li RongQing wrote:
> diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
> index 6613d8dbb0e5..b73d40d68178 100644
> --- a/net/ipv6/ip6_fib.c
> +++ b/net/ipv6/ip6_fib.c
> @@ -1143,7 +1143,7 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct fib6_info *rt,
>
> atomic_inc(&rt->fib6_ref);
> rcu_assign_pointer(rt->fib6_node, fn);
> - rt->fib6_next = iter->fib6_next;
> + rcu_assign_pointer(rt->fib6_next, iter->fib6_next);
> rcu_assign_pointer(*ins, rt);
> if (!info->skip_notify)
> inet6_rt_notify(RTM_NEWROUTE, rt, info, NLM_F_REPLACE);
> @@ -1761,7 +1761,9 @@ static void fib6_del_route(struct fib6_table *table, struct fib6_node *fn,
> RT6_TRACE("fib6_del_route\n");
>
> /* Unlink it */
> - *rtp = rt->fib6_next;
> + *rtp = rcu_dereference_protected(rt->fib6_next,
> + lockdep_is_held(&rt->fib6_table->tb6_lock));
> +
> rt->fib6_node = NULL;
> net->ipv6.rt6_stats->fib_rt_entries--;
> net->ipv6.rt6_stats->fib_discarded_routes++;
>
There are a number of fib6_next references not using rcu api's. Why are
you only touching these 2?
^ permalink raw reply
* [PATCH bpf 0/2] nfp: bpf: fix two ALU32 code-gen bugs
From: Jiong Wang @ 2019-02-22 22:36 UTC (permalink / raw)
To: alexei.starovoitov, daniel; +Cc: netdev, oss-drivers, Jiong Wang
code-gen for BPF_ALU | BPF_XOR | BPF_K is wrong when imm is -1, also high
32-bit of 64-bit register should always be cleared.
This set fixed both bugs.
Jiong Wang (2):
nfp: bpf: fix code-gen bug on BPF_ALU | BPF_XOR | BPF_K
nfp: bpf: fix ALU32 high bits clearance bug
drivers/net/ethernet/netronome/nfp/bpf/jit.c | 17 ++++++-----------
1 file changed, 6 insertions(+), 11 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH bpf 1/2] nfp: bpf: fix code-gen bug on BPF_ALU | BPF_XOR | BPF_K
From: Jiong Wang @ 2019-02-22 22:36 UTC (permalink / raw)
To: alexei.starovoitov, daniel; +Cc: netdev, oss-drivers, Jiong Wang
In-Reply-To: <1550874964-16769-1-git-send-email-jiong.wang@netronome.com>
The intended optimization should be A ^ 0 = A, not A ^ -1 = A.
Fixes: cd7df56ed3e6 ("nfp: add BPF to NFP code translator")
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Signed-off-by: Jiong Wang <jiong.wang@netronome.com>
---
drivers/net/ethernet/netronome/nfp/bpf/jit.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/jit.c b/drivers/net/ethernet/netronome/nfp/bpf/jit.c
index e23ca90..a096965 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/jit.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/jit.c
@@ -2309,7 +2309,7 @@ static int xor_reg(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
static int xor_imm(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
{
- return wrp_alu32_imm(nfp_prog, meta, ALU_OP_XOR, !~meta->insn.imm);
+ return wrp_alu32_imm(nfp_prog, meta, ALU_OP_XOR, !meta->insn.imm);
}
static int and_reg(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
--
2.7.4
^ permalink raw reply related
* [PATCH bpf 2/2] nfp: bpf: fix ALU32 high bits clearance bug
From: Jiong Wang @ 2019-02-22 22:36 UTC (permalink / raw)
To: alexei.starovoitov, daniel; +Cc: netdev, oss-drivers, Jiong Wang
In-Reply-To: <1550874964-16769-1-git-send-email-jiong.wang@netronome.com>
NFP BPF JIT compiler is doing a couple of small optimizations when jitting
ALU imm instructions, some of these optimizations could save code-gen, for
example:
A & -1 = A
A | 0 = A
A ^ 0 = A
However, for ALU32, high 32-bit of the 64-bit register should still be
cleared according to ISA semantics.
Fixes: cd7df56ed3e6 ("nfp: add BPF to NFP code translator")
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Signed-off-by: Jiong Wang <jiong.wang@netronome.com>
---
drivers/net/ethernet/netronome/nfp/bpf/jit.c | 17 ++++++-----------
1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/jit.c b/drivers/net/ethernet/netronome/nfp/bpf/jit.c
index a096965..0a868c8 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/jit.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/jit.c
@@ -1291,15 +1291,10 @@ wrp_alu64_reg(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta,
static int
wrp_alu32_imm(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta,
- enum alu_op alu_op, bool skip)
+ enum alu_op alu_op)
{
const struct bpf_insn *insn = &meta->insn;
- if (skip) {
- meta->skip = true;
- return 0;
- }
-
wrp_alu_imm(nfp_prog, insn->dst_reg * 2, alu_op, insn->imm);
wrp_immed(nfp_prog, reg_both(insn->dst_reg * 2 + 1), 0);
@@ -2309,7 +2304,7 @@ static int xor_reg(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
static int xor_imm(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
{
- return wrp_alu32_imm(nfp_prog, meta, ALU_OP_XOR, !meta->insn.imm);
+ return wrp_alu32_imm(nfp_prog, meta, ALU_OP_XOR);
}
static int and_reg(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
@@ -2319,7 +2314,7 @@ static int and_reg(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
static int and_imm(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
{
- return wrp_alu32_imm(nfp_prog, meta, ALU_OP_AND, !~meta->insn.imm);
+ return wrp_alu32_imm(nfp_prog, meta, ALU_OP_AND);
}
static int or_reg(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
@@ -2329,7 +2324,7 @@ static int or_reg(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
static int or_imm(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
{
- return wrp_alu32_imm(nfp_prog, meta, ALU_OP_OR, !meta->insn.imm);
+ return wrp_alu32_imm(nfp_prog, meta, ALU_OP_OR);
}
static int add_reg(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
@@ -2339,7 +2334,7 @@ static int add_reg(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
static int add_imm(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
{
- return wrp_alu32_imm(nfp_prog, meta, ALU_OP_ADD, !meta->insn.imm);
+ return wrp_alu32_imm(nfp_prog, meta, ALU_OP_ADD);
}
static int sub_reg(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
@@ -2349,7 +2344,7 @@ static int sub_reg(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
static int sub_imm(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
{
- return wrp_alu32_imm(nfp_prog, meta, ALU_OP_SUB, !meta->insn.imm);
+ return wrp_alu32_imm(nfp_prog, meta, ALU_OP_SUB);
}
static int mul_reg(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] selftests: fib_tests: sleep after changing carrier. again.
From: David Ahern @ 2019-02-22 22:37 UTC (permalink / raw)
To: Thadeu Lima de Souza Cascardo, netdev; +Cc: linux-kselftest, David Ahern
In-Reply-To: <20190222102741.31173-1-cascardo@canonical.com>
On 2/22/19 5:27 AM, Thadeu Lima de Souza Cascardo wrote:
> Just like commit e2ba732a1681 ("selftests: fib_tests: sleep after
> changing carrier"), wait one second to allow linkwatch to propagate the
> carrier change to the stack.
>
> There are two sets of carrier tests. The first slept after the carrier
> was set to off, and when the second set ran, it was likely that the
> linkwatch would be able to run again without much delay, reducing the
> likelihood of a race. However, if you run 'fib_tests.sh -t carrier' on a
> loop, you will quickly notice the failures.
>
> Sleeping on the second set of tests make the failures go away.
>
> Cc: David Ahern <dsahern@gmail.com>
> Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@canonical.com>
> ---
> tools/testing/selftests/net/fib_tests.sh | 1 +
> 1 file changed, 1 insertion(+)
>
Reviewed-by: David Ahern <dsahern@gmail.com>
^ permalink raw reply
* Re: [PATCH 1/2 v2] kprobe: Do not use uaccess functions to access kernel memory that can fault
From: Nadav Amit @ 2019-02-22 22:39 UTC (permalink / raw)
To: Jann Horn
Cc: Andy Lutomirski, Alexei Starovoitov, Steven Rostedt,
Linus Torvalds, Masami Hiramatsu, Linux List Kernel Mailing,
Ingo Molnar, Andrew Morton, Changbin Du, Kees Cook,
Andy Lutomirski, Daniel Borkmann, Network Development,
bpf@vger.kernel.org, Rick Edgecombe, Dave Hansen,
Peter Zijlstra (Intel), Igor Stoppa
In-Reply-To: <7701651F-F10E-4212-925E-1CB77C5D3E69@vmware.com>
> On Feb 22, 2019, at 2:21 PM, Nadav Amit <namit@vmware.com> wrote:
>
>> On Feb 22, 2019, at 2:17 PM, Jann Horn <jannh@google.com> wrote:
>>
>> On Fri, Feb 22, 2019 at 11:08 PM Nadav Amit <namit@vmware.com> wrote:
>>>> On Feb 22, 2019, at 1:43 PM, Jann Horn <jannh@google.com> wrote:
>>>>
>>>> (adding some people from the text_poke series to the thread, removing stable@)
>>>>
>>>> On Fri, Feb 22, 2019 at 8:55 PM Andy Lutomirski <luto@amacapital.net> wrote:
>>>>>> On Feb 22, 2019, at 11:34 AM, Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
>>>>>>> On Fri, Feb 22, 2019 at 02:30:26PM -0500, Steven Rostedt wrote:
>>>>>>> On Fri, 22 Feb 2019 11:27:05 -0800
>>>>>>> Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
>>>>>>>
>>>>>>>>> On Fri, Feb 22, 2019 at 09:43:14AM -0800, Linus Torvalds wrote:
>>>>>>>>>
>>>>>>>>> Then we should still probably fix up "__probe_kernel_read()" to not
>>>>>>>>> allow user accesses. The easiest way to do that is actually likely to
>>>>>>>>> use the "unsafe_get_user()" functions *without* doing a
>>>>>>>>> uaccess_begin(), which will mean that modern CPU's will simply fault
>>>>>>>>> on a kernel access to user space.
>>>>>>>>
>>>>>>>> On bpf side the bpf_probe_read() helper just calls probe_kernel_read()
>>>>>>>> and users pass both user and kernel addresses into it and expect
>>>>>>>> that the helper will actually try to read from that address.
>>>>>>>>
>>>>>>>> If __probe_kernel_read will suddenly start failing on all user addresses
>>>>>>>> it will break the expectations.
>>>>>>>> How do we solve it in bpf_probe_read?
>>>>>>>> Call probe_kernel_read and if that fails call unsafe_get_user byte-by-byte
>>>>>>>> in the loop?
>>>>>>>> That's doable, but people already complain that bpf_probe_read() is slow
>>>>>>>> and shows up in their perf report.
>>>>>>>
>>>>>>> We're changing kprobes to add a specific flag to say that we want to
>>>>>>> differentiate between kernel or user reads. Can this be done with
>>>>>>> bpf_probe_read()? If it's showing up in perf report, I doubt a single
>>>>>>
>>>>>> so you're saying you will break existing kprobe scripts?
>>>>>> I don't think it's a good idea.
>>>>>> It's not acceptable to break bpf_probe_read uapi.
>>>>>
>>>>> If so, the uapi is wrong: a long-sized number does not reliably identify an address if you don’t separately know whether it’s a user or kernel address. s390x and 4G:4G x86_32 are the notable exceptions. I have lobbied for RISC-V and future x86_64 to join the crowd. I don’t know whether I’ll win this fight, but the uapi will probably have to change for at least s390x.
>>>>>
>>>>> What to do about existing scripts is a different question.
>>>>
>>>> This lack of logical separation between user and kernel addresses
>>>> might interact interestingly with the text_poke series, specifically
>>>> "[PATCH v3 05/20] x86/alternative: Initialize temporary mm for
>>>> patching" (https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flkml%2F20190221234451.17632-6-rick.p.edgecombe%40intel.com%2F&data=02%7C01%7Cnamit%40vmware.com%7Cf2513009ef734ecd6b0d08d69913a5ae%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636864707020821793&sdata=HAbnDcrBne64JyPuVUMKmM7nQk67F%2BFvjuXEn8TmHeo%3D&reserved=0)
>>>> and "[PATCH v3 06/20] x86/alternative: Use temporary mm for text
>>>> poking" (https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flkml%2F20190221234451.17632-7-rick.p.edgecombe%40intel.com%2F&data=02%7C01%7Cnamit%40vmware.com%7Cf2513009ef734ecd6b0d08d69913a5ae%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636864707020821793&sdata=vNRIMKtFDy%2F3z5FlTwDiJY6VGEV%2FMHgQPTdFSFtCo4s%3D&reserved=0),
>>>> right? If someone manages to get a tracing BPF program to trigger in a
>>>> task that has switched to the patching mm, could they use
>>>> bpf_probe_write_user() - which uses probe_kernel_write() after
>>>> checking that KERNEL_DS isn't active and that access_ok() passes - to
>>>> overwrite kernel text that is mapped writable in the patching mm?
>>>
>>> Yes, this is a good point. I guess text_poke() should be defined with
>>> “__kprobes” and open-code memcpy.
>>>
>>> Does it sound reasonable?
>>
>> Doesn't __text_poke() as implemented in the proposed patch use a
>> couple other kernel functions, too? Like switch_mm_irqs_off() and
>> pte_clear() (which can be a call into a separate function on paravirt
>> kernels)?
>
> I will move the pte_clear() to be done after the poking mm was unloaded.
> Give me a few minutes to send a sketch of what I think should be done.
Err.. You are right, I don’t see an easy way of preventing a kprobe from
being set on switch_mm_irqs_off(), and open-coding this monster is too ugly.
The reasonable solution seems to me as taking all the relevant pieces of
code (and data) that might be used during text-poking and encapsulating them, so they
will be set in a memory area which cannot be kprobe'd. This can also be
useful to write-protect data structures of code that calls text_poke(),
e.g., static-keys. It can also protect data on that stack that is used
during text_poke() from being overwritten from another core.
This solution is somewhat similar to Igor Stoppa’s idea of using “enclaves”
when doing write-rarely operations.
Right now, I think that text_poke() will keep being susceptible to such
an attack, unless you have a better suggestion.
^ permalink raw reply
* [PATCH net-next 0/5] net: phy: aquantia: improve and extend driver
From: Heiner Kallweit @ 2019-02-22 22:45 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev@vger.kernel.org
This series improves and extends the Aquantia PHY driver.
Andrew Lunn (1):
net: phy: aquantia: use genphy_c45_an_config_aneg
Heiner Kallweit (4):
net: phy: aquantia: remove false 5G and 10G speed ability for AQCS109
net: phy: don't change modes we don't care about in genphy_c45_read_lpa
net: phy: add genphy_c45_read_status
net: phy: aquantia: use genphy_c45_read_status
drivers/net/phy/aquantia.c | 94 ++++++++++++++++++++++++++------------
drivers/net/phy/phy-c45.c | 35 +++++++++++++-
include/linux/phy.h | 1 +
3 files changed, 99 insertions(+), 31 deletions(-)
--
2.20.1
^ 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