* [PATH RFC net-next 5/8] net: phy: Add limkmode equivalents to some of the MII ethtool helpers
From: Andrew Lunn @ 2018-09-14 21:38 UTC (permalink / raw)
To: netdev; +Cc: Florian Fainelli, Andrew Lunn
In-Reply-To: <1536961136-30453-1-git-send-email-andrew@lunn.ch>
Add helpers which take a linkmode rather than a u32 ethtool for
advertising settings.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
include/linux/mii.h | 50 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 50 insertions(+)
diff --git a/include/linux/mii.h b/include/linux/mii.h
index 9ed49c8261d0..2da85b02e1c0 100644
--- a/include/linux/mii.h
+++ b/include/linux/mii.h
@@ -132,6 +132,34 @@ static inline u32 ethtool_adv_to_mii_adv_t(u32 ethadv)
return result;
}
+/**
+ * linkmode_adv_to_mii_adv_t
+ * @advertising: the linkmode advertisement settings
+ *
+ * A small helper function that translates linkmode advertisement
+ * settings to phy autonegotiation advertisements for the
+ * MII_ADVERTISE register.
+ */
+static inline u32 linkmode_adv_to_mii_adv_t(unsigned long *advertising)
+{
+ u32 result = 0;
+
+ if (linkmode_test_bit(ETHTOOL_LINK_MODE_10baseT_Half_BIT, advertising))
+ result |= ADVERTISE_10HALF;
+ if (linkmode_test_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT, advertising))
+ result |= ADVERTISE_10FULL;
+ if (linkmode_test_bit(ETHTOOL_LINK_MODE_100baseT_Half_BIT, advertising))
+ result |= ADVERTISE_100HALF;
+ if (linkmode_test_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT, advertising))
+ result |= ADVERTISE_100FULL;
+ if (linkmode_test_bit(ETHTOOL_LINK_MODE_Pause_BIT, advertising))
+ result |= ADVERTISE_PAUSE_CAP;
+ if (linkmode_test_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, advertising))
+ result |= ADVERTISE_PAUSE_ASYM;
+
+ return result;
+}
+
/**
* mii_adv_to_ethtool_adv_t
* @adv: value of the MII_ADVERTISE register
@@ -179,6 +207,28 @@ static inline u32 ethtool_adv_to_mii_ctrl1000_t(u32 ethadv)
return result;
}
+/**
+ * linkmode_adv_to_mii_ctrl1000_t
+ * advertising: the linkmode advertisement settings
+ *
+ * A small helper function that translates linkmode advertisement
+ * settings to phy autonegotiation advertisements for the
+ * MII_CTRL1000 register when in 1000T mode.
+ */
+static inline u32 linkmode_adv_to_mii_ctrl1000_t(unsigned long *advertising)
+{
+ u32 result = 0;
+
+ if (linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseT_Half_BIT,
+ advertising))
+ result |= ADVERTISE_1000HALF;
+ if (linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
+ advertising))
+ result |= ADVERTISE_1000FULL;
+
+ return result;
+}
+
/**
* mii_ctrl1000_to_ethtool_adv_t
* @adv: value of the MII_CTRL1000 register
--
2.19.0.rc1
^ permalink raw reply related
* Re: [PATCH net-next v3 0/2] net: stmmac: Coalesce and tail addr fixes
From: David Miller @ 2018-09-14 21:27 UTC (permalink / raw)
To: Jose.Abreu
Cc: netdev, f.fainelli, narmstrong, jbrunet, martin.blumenstingl,
Joao.Pinto, peppe.cavallaro, alexandre.torgue
In-Reply-To: <cover.1536762575.git.joabreu@synopsys.com>
From: Jose Abreu <Jose.Abreu@synopsys.com>
Date: Thu, 13 Sep 2018 09:02:21 +0100
> The fix for coalesce timer and a fix in tail address setting that impacts
> XGMAC2 operation.
This series is fixing bugs going all the way back to 4.7
There is no logical way that targetting net-next is valid.
net-next is always for new features and cleanups.
Bug fixes always go to 'net'.
Thank you.
^ permalink raw reply
* mlx5_core: null pointer dereference in mlx5_accel_tls_device_caps() (net-next kernel)
From: Michal Kubecek @ 2018-09-14 21:20 UTC (permalink / raw)
To: netdev; +Cc: Saeed Mahameed, Leon Romanovsky
I just encountered a null pointer dereference on mlx5_core module
initialization while booting net-next kernel (based on commit
ee4fccbee7d3) on an aarch64 machine:
[ 12.021971] iommu: Adding device 0000:01:00.0 to group 3
[ 12.022925] mlx5_core 0000:01:00.0: firmware version: 12.17.2020
[ 12.022954] mlx5_core 0000:01:00.0: 63.008 Gb/s available PCIe bandwidth (8 GT/s x8 link)
[ 12.068709] Adding 98830144k swap on /dev/sda4. Priority:-2 extents:1 across:98830144k FS
[ 12.347571] (0000:01:00.0): E-Switch: Total vports 9, per vport: max uc(1024) max mc(16384)
[ 12.351962] mlx5_core 0000:01:00.0: Port module event: module 0, Cable plugged
[ 12.366306] mlx5_core 0000:01:00.0: MLX5E: StrdRq(0) RqSz(1024) StrdSz(128) RxCqeCmprss(0)
[ 12.366741] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000050
[ 12.374603] Mem abort info:
[ 12.377368] ESR = 0x96000004
[ 12.380406] Exception class = DABT (current EL), IL = 32 bits
[ 12.386357] SET = 0, FnV = 0
[ 12.389347] EA = 0, S1PTW = 0
[ 12.392471] Data abort info:
[ 12.395343] ISV = 0, ISS = 0x00000004
[ 12.399156] CM = 0, WnR = 0
[ 12.402108] user pgtable: 4k pages, 48-bit VAs, pgdp = (____ptrval____)
[ 12.408711] [0000000000000050] pgd=0000000000000000
[ 12.413567] Internal error: Oops: 96000004 [#1] SMP
[ 12.418427] Modules linked in: fat mlx5_core(+) ipmi_ssif(+) aes_ce_blk crypto_simd cryptd aes_ce_cipher crc32_ce crct10dif_ce ghash_ce aes_arm64 sha2_ce sha256_arm64 sha1_ce ipmi_devintf ipmi_msghandler sbsa_gwdt tls mlxfw devlink at803x qcom_emac btrfs libcrc32c xor zlib_deflate raid6_pq ahci_platform libahci_platform hdma hdma_mgmt i2c_qup sg dm_multipath dm_mod scsi_dh_rdac scsi_dh_emc scsi_dh_alua efivarfs
[ 12.454800] CPU: 40 PID: 742 Comm: systemd-udevd Not tainted 4.19.0-rc3-ethnl.15-default #1
[ 12.463131] Hardware name: To be filled by O.E.M. To be filled by O.E.M./To be filled by O.E.M., BIOS 5.13 12/12/2012
[ 12.473722] pstate: 60400005 (nZCv daif +PAN -UAO)
[ 12.478559] pc : mlx5_accel_tls_device_caps+0x28/0x38 [mlx5_core]
[ 12.484598] lr : mlx5e_tls_build_netdev+0x24/0x98 [mlx5_core]
[ 12.490301] sp : ffff000021873a30
[ 12.493599] x29: ffff000021873a30 x28: ffff2a72560a7940
[ 12.498895] x27: ffff2a7256df6000 x26: ffff2a71a0fed650
[ 12.504190] x25: 0000000000000000 x24: ffff92c7f2b988c0
[ 12.509485] x23: ffff92c7fe01c0c0 x22: ffff2a71a0fcfa70
[ 12.514780] x21: ffff92c7f2b808c0 x20: ffff92c7f741c110
[ 12.520075] x19: ffff92c7f2b988c0 x18: ffff0000218739b0
[ 12.525370] x17: 0000000000000000 x16: ffff2a725625ade0
[ 12.530665] x15: 0000000029818ed4 x14: 00000000d47aab07
[ 12.535961] x13: 8a24000000000000 x12: 0000000000000000
[ 12.541256] x11: 0000000000000000 x10: 0000000000000000
[ 12.546551] x9 : 0000000000000000 x8 : 0000000000000000
[ 12.551846] x7 : 0000000000000000 x6 : ffff92c8159dc910
[ 12.557141] x5 : 0000000000000400 x4 : ffff7e4b205a20c7
[ 12.562436] x3 : 0000000000000000 x2 : ffff2a725625ae1c
[ 12.567731] x1 : 00000000ab078a24 x0 : 0000000000000000
[ 12.573027] Process systemd-udevd (pid: 742, stack limit = 0x(____ptrval____))
[ 12.580232] Call trace:
[ 12.582688] mlx5_accel_tls_device_caps+0x28/0x38 [mlx5_core]
[ 12.588419] mlx5e_build_nic_netdev+0x27c/0x348 [mlx5_core]
[ 12.593974] mlx5e_nic_init+0x1a0/0x258 [mlx5_core]
[ 12.598835] mlx5e_create_netdev+0x74/0x118 [mlx5_core]
[ 12.604043] mlx5e_add+0xf0/0x2c0 [mlx5_core]
[ 12.608384] mlx5_add_device+0x88/0x1a8 [mlx5_core]
[ 12.613246] mlx5_register_interface+0x78/0xb0 [mlx5_core]
[ 12.618713] mlx5e_init+0x24/0x30 [mlx5_core]
[ 12.623052] init+0x88/0xa0 [mlx5_core]
[ 12.626850] do_one_initcall+0x54/0x200
[ 12.630667] do_init_module+0x64/0x1d8
[ 12.634401] load_module+0x1480/0x1510
[ 12.638132] __se_sys_finit_module+0xc8/0xd8
[ 12.642385] __arm64_sys_finit_module+0x24/0x30
[ 12.646901] el0_svc_common+0x7c/0x118
[ 12.650631] el0_svc_handler+0x38/0x78
[ 12.654364] el0_svc+0x8/0xc
[ 12.657229] Code: d503201f f97c7e60 f9400bf3 a8c27bfd (f9402800)
[ 12.663306] ---[ end trace 57e772dd3cf718f1 ]---
The function looks like this:
------------------------------------------------------------------------
drivers/net/ethernet/mellanox/mlx5/core/accel/tls.c:
68 {
0x0000000000058230 <+0>: stp x29, x30, [sp, #-32]!
0x0000000000058234 <+4>: mov x29, sp
0x0000000000058238 <+8>: str x19, [sp, #16]
0x000000000005823c <+12>: mov x19, x0
0x0000000000058240 <+16>: mov x0, x30
drivers/net/ethernet/mellanox/mlx5/core/fpga/tls.h:
68 return mdev->fpga->tls->caps;
0x0000000000058244 <+20>: add x19, x19, #0x38, lsl #12
drivers/net/ethernet/mellanox/mlx5/core/accel/tls.c:
68 {
0x0000000000058248 <+24>: bl 0x58248
<mlx5_accel_tls_device_caps+24>
drivers/net/ethernet/mellanox/mlx5/core/fpga/tls.h:
68 return mdev->fpga->tls->caps;
0x000000000005824c <+28>: ldr x0, [x19, #30968]
drivers/net/ethernet/mellanox/mlx5/core/accel/tls.c:
70 }
0x0000000000058250 <+32>: ldr x19, [sp, #16]
0x0000000000058254 <+36>: ldp x29, x30, [sp], #32
drivers/net/ethernet/mellanox/mlx5/core/fpga/tls.h:
68 return mdev->fpga->tls->caps;
0x0000000000058258 <+40>: ldr x0, [x0, #80]
drivers/net/ethernet/mellanox/mlx5/core/accel/tls.c:
70 }
0x000000000005825c <+44>: ldr w0, [x0, #20]
0x0000000000058260 <+48>: ret
------------------------------------------------------------------------
so IIUC mdev->fpga is null (offset of tls in struct mlx5_fpga_device is
indeed 80 = 0x50).
The NIC is
Model: "Mellanox MT27700 Family [ConnectX-4]"
Vendor: pci 0x15b3 "Mellanox Technologies"
Device: pci 0x1013 "MT27700 Family [ConnectX-4]"
SubVendor: pci 0x15b3 "Mellanox Technologies"
SubDevice: pci 0x0003
Michal Kubecek
^ permalink raw reply
* Re: [PATCH net-next v3 09/11] dt-bindings: add constants for Microsemi Ocelot SerDes driver
From: Florian Fainelli @ 2018-09-15 2:31 UTC (permalink / raw)
To: Quentin Schulz, alexandre.belloni, ralf, paul.burton, jhogan,
robh+dt, mark.rutland, davem, kishon, andrew
Cc: allan.nielsen, linux-mips, devicetree, linux-kernel, netdev,
thomas.petazzoni
In-Reply-To: <73113ea4b3d8d34c05d88413b3d15cc1733cf25e.1536912834.git-series.quentin.schulz@bootlin.com>
On 09/14/18 01:16, Quentin Schulz wrote:
> The Microsemi Ocelot has multiple SerDes and requires that the SerDes be
> muxed accordingly to the hardware representation.
>
> Let's add a constant for each SerDes available in the Microsemi Ocelot.
>
> Signed-off-by: Quentin Schulz <quentin.schulz@bootlin.com>
> ---
> include/dt-bindings/phy/phy-ocelot-serdes.h | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
> create mode 100644 include/dt-bindings/phy/phy-ocelot-serdes.h
>
> diff --git a/include/dt-bindings/phy/phy-ocelot-serdes.h b/include/dt-bindings/phy/phy-ocelot-serdes.h
> new file mode 100644
> index 0000000..cf111ba
> --- /dev/null
> +++ b/include/dt-bindings/phy/phy-ocelot-serdes.h
> @@ -0,0 +1,19 @@
> +/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
> +/* Copyright (c) 2018 Microsemi Corporation */
> +#ifndef __PHY_OCELOT_SERDES_H__
> +#define __PHY_OCELOT_SERDES_H__
> +
> +#define SERDES1G_0 0
> +#define SERDES1G_1 1
> +#define SERDES1G_2 2
> +#define SERDES1G_3 3
> +#define SERDES1G_4 4
> +#define SERDES1G_5 5
> +#define SERDES1G_MAX 6
Given you use the C preprocessor you could have done something like:
#define SERDES1G(x) (x)
#define SERDES1G_MAX 5
#define SERDES6G(x) ((x) + SERDES1G_MAX)
etc. but this works for me as well.
> +#define SERDES6G_0 SERDES1G_MAX
> +#define SERDES6G_1 (SERDES1G_MAX + 1)
> +#define SERDES6G_2 (SERDES1G_MAX + 2)
> +#define SERDES6G_MAX (SERDES1G_MAX + 3)
> +#define SERDES_MAX (SERDES1G_MAX + SERDES6G_MAX)
> +
> +#endif
>
--
Florian
^ permalink raw reply
* Re: [PATCH net-next v3 08/11] MIPS: mscc: ocelot: add SerDes mux DT node
From: Florian Fainelli @ 2018-09-15 2:30 UTC (permalink / raw)
To: Quentin Schulz, alexandre.belloni, ralf, paul.burton, jhogan,
robh+dt, mark.rutland, davem, kishon, andrew
Cc: allan.nielsen, linux-mips, devicetree, linux-kernel, netdev,
thomas.petazzoni
In-Reply-To: <ba95add3d931177ef55666e856164523903d1148.1536912834.git-series.quentin.schulz@bootlin.com>
On 09/14/18 01:16, Quentin Schulz wrote:
> The Microsemi Ocelot has a set of register for SerDes/switch port muxing
> as well as PCIe muxing for a specific SerDes, so let's add the device
> and all SerDes in the Device Tree.
>
> Acked-by: Paul Burton <paul.burton@mips.com>
> Signed-off-by: Quentin Schulz <quentin.schulz@bootlin.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* Re: [PATCH net-next v3 07/11] dt-bindings: phy: add DT binding for Microsemi Ocelot SerDes muxing
From: Florian Fainelli @ 2018-09-15 2:29 UTC (permalink / raw)
To: Quentin Schulz, alexandre.belloni, ralf, paul.burton, jhogan,
robh+dt, mark.rutland, davem, kishon, andrew
Cc: allan.nielsen, linux-mips, devicetree, linux-kernel, netdev,
thomas.petazzoni
In-Reply-To: <f392dafca9165800439fc09cd7d16e6a9506d457.1536912834.git-series.quentin.schulz@bootlin.com>
On 09/14/18 01:16, Quentin Schulz wrote:
> Signed-off-by: Quentin Schulz <quentin.schulz@bootlin.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* Re: [PATCH net-next v3 06/11] phy: add QSGMII and PCIE modes
From: Florian Fainelli @ 2018-09-15 2:27 UTC (permalink / raw)
To: Quentin Schulz, alexandre.belloni, ralf, paul.burton, jhogan,
robh+dt, mark.rutland, davem, kishon, andrew
Cc: allan.nielsen, linux-mips, devicetree, linux-kernel, netdev,
thomas.petazzoni
In-Reply-To: <52d9c9444175911a8f6ee3dfec8946646907135b.1536912834.git-series.quentin.schulz@bootlin.com>
On 09/14/18 01:16, Quentin Schulz wrote:
> Prepare for upcoming phys that'll handle QSGMII or PCIe.
>
> Signed-off-by: Quentin Schulz <quentin.schulz@bootlin.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* Re: [PATCH net-next v3 05/11] net: mscc: ocelot: simplify register access for PLL5 configuration
From: Florian Fainelli @ 2018-09-15 2:26 UTC (permalink / raw)
To: Quentin Schulz, alexandre.belloni, ralf, paul.burton, jhogan,
robh+dt, mark.rutland, davem, kishon, andrew
Cc: allan.nielsen, linux-mips, devicetree, linux-kernel, netdev,
thomas.petazzoni
In-Reply-To: <b09eb1026a2870c6cc882997ac69e25fdf7a96d8.1536912834.git-series.quentin.schulz@bootlin.com>
On 09/14/18 01:16, Quentin Schulz wrote:
> Since HSIO address space can be accessed by different drivers, let's
> simplify the register address definitions so that it can be easily used
> by all drivers and put the register address definition in the
> include/soc/mscc/ocelot_hsio.h header file.
>
> Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Signed-off-by: Quentin Schulz <quentin.schulz@bootlin.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* Re: [net-next,RFC PATCH] Introduce TC Range classifier
From: Cong Wang @ 2018-09-14 21:09 UTC (permalink / raw)
To: Jiri Pirko
Cc: amritha.nambiar, Linux Kernel Network Developers, David Miller,
Alexander Duyck, Jakub Kicinski, sridhar.samudrala,
Jamal Hadi Salim, Jesse Brandeburg
In-Reply-To: <20180914094932.GK25110@nanopsycho>
On Fri, Sep 14, 2018 at 2:53 AM Jiri Pirko <jiri@resnulli.us> wrote:
>
> Thu, Sep 13, 2018 at 10:52:01PM CEST, amritha.nambiar@intel.com wrote:
> >This patch introduces a TC range classifier to support filtering based
> >on ranges. Only port-range filters are supported currently. This can
> >be combined with flower classifier to support filters that are a
> >combination of port-ranges and other parameters based on existing
> >fields supported by cls_flower. The 'goto chain' action can be used to
> >combine the flower and range filter.
> >The filter precedence is decided based on the 'prio' value.
>
> For example Spectrum ASIC supports mask-based and range-based matching
> in a single TCAM rule. No chains needed. Also, I don't really understand
> why is this a separate cls. I believe that this functionality should be
> put as an extension of existing cls_flower.
Exactly. u32 filters support range matching too with proper masks.
^ permalink raw reply
* Re: [PATCH net-next v3 04/11] net: mscc: ocelot: move the HSIO header to include/soc
From: Florian Fainelli @ 2018-09-15 2:24 UTC (permalink / raw)
To: Quentin Schulz, alexandre.belloni, ralf, paul.burton, jhogan,
robh+dt, mark.rutland, davem, kishon, andrew
Cc: allan.nielsen, linux-mips, devicetree, linux-kernel, netdev,
thomas.petazzoni
In-Reply-To: <45bc0a8bd6a1dfc35adcedc3581124879fdb5a07.1536912834.git-series.quentin.schulz@bootlin.com>
On 09/14/18 01:16, Quentin Schulz wrote:
> Since HSIO address space can be used by different drivers (PLL, SerDes
> muxing, temperature sensor), let's move it somewhere it can be included
> by all drivers.
>
> Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Signed-off-by: Quentin Schulz <quentin.schulz@bootlin.com>
Nothing wrong with the patch, you likely would have wanted to use git
format-patch -M such that the diff would have been showing that the file
was renamed.
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* Re: [PATCH net-next v3 03/11] net: mscc: ocelot: get HSIO regmap from syscon
From: Florian Fainelli @ 2018-09-15 2:23 UTC (permalink / raw)
To: Quentin Schulz, alexandre.belloni, ralf, paul.burton, jhogan,
robh+dt, mark.rutland, davem, kishon, andrew
Cc: allan.nielsen, linux-mips, devicetree, linux-kernel, netdev,
thomas.petazzoni
In-Reply-To: <d9b23949e81272006e076e0b58d90e0541f80c7f.1536912834.git-series.quentin.schulz@bootlin.com>
On 09/14/18 01:16, Quentin Schulz wrote:
> HSIO address space was moved to a syscon, hence we need to get the
> regmap of this address space from there and no more from the device
> node.
>
> Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Signed-off-by: Quentin Schulz <quentin.schulz@bootlin.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* Re: [net-next, RFC PATCH] net: sched: cls_range: Introduce Range classifier
From: Cong Wang @ 2018-09-14 21:06 UTC (permalink / raw)
To: amritha.nambiar
Cc: Linux Kernel Network Developers, David Miller, Alexander Duyck,
Jakub Kicinski, sridhar.samudrala, Jamal Hadi Salim,
Jesse Brandeburg, Jiri Pirko
In-Reply-To: <153687192654.43503.1433255216543560934.stgit@anamhost.jf.intel.com>
On Thu, Sep 13, 2018 at 6:53 PM Amritha Nambiar
<amritha.nambiar@intel.com> wrote:
>
> This patch introduces a range classifier to support filtering based
> on ranges. Only port-range filters are supported currently. This can
> be combined with flower classifier to support filters that are a
> combination of port-ranges and other parameters based on existing
> fields supported by cls_flower.
Why should we have a special-purpose filter just for ports here?
We have achieved almost the same goal with u32 filter:
https://github.com/apache/mesos/blob/master/src/slave/containerizer/mesos/isolators/network/port_mapping.cpp
There is a large overlap with other general purpose filters.
I don't see you provide any justification for the purpose of it. If
it is just for convenience, can't we just make it on top of other
general purpose header-matching filters?
^ permalink raw reply
* Re: [PATCH net-next 2/5] net: phy: mscc: Add EEE init sequence
From: Florian Fainelli @ 2018-09-15 2:21 UTC (permalink / raw)
To: Quentin Schulz, davem, andrew
Cc: allan.nielsen, linux-kernel, netdev, thomas.petazzoni,
Raju Lakkaraju
In-Reply-To: <64809c5f01f3c6407257553a286b82949cef1ac0.1536913944.git-series.quentin.schulz@bootlin.com>
On 09/14/18 01:33, Quentin Schulz wrote:
> From: Raju Lakkaraju <Raju.Lakkaraju@microchip.com>
>
> Microsemi PHYs (VSC 8530/31/40/41) need to update the Energy Efficient
> Ethernet initialization sequence.
> In order to avoid certain link state errors that could result in link
> drops and packet loss, the physical coding sublayer (PCS) must be
> updated with settings related to EEE in order to improve performance.
>
> Signed-off-by: Raju Lakkaraju <Raju.Lakkaraju@microchip.com>
> Signed-off-by: Quentin Schulz <quentin.schulz@bootlin.com>
> ---
[snip]
> + vsc85xx_tr_write(phydev, 0x0f82, 0x0012b00a);
Can you just make this an array of register + value pair? That would be
less error prone in case you need to update that sequence in the future.
With that:
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* Re: [PATCH net-next 5/5] net: phy: mscc: remove unneeded temporary variable
From: Florian Fainelli @ 2018-09-15 2:19 UTC (permalink / raw)
To: Quentin Schulz, davem, andrew
Cc: allan.nielsen, linux-kernel, netdev, thomas.petazzoni
In-Reply-To: <d9cca8eef36bb8918c9ed28574b79b7674fd36f6.1536913944.git-series.quentin.schulz@bootlin.com>
On 09/14/18 01:33, Quentin Schulz wrote:
> Here, the rc variable is either used only for the condition right after
> the assignment or right before being used as the return value of the
> function it's being used in.
>
> So let's remove this unneeded temporary variable whenever possible.
>
> Signed-off-by: Quentin Schulz <quentin.schulz@bootlin.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* [PATCH net] tls: fix currently broken MSG_PEEK behavior
From: Daniel Borkmann @ 2018-09-14 21:00 UTC (permalink / raw)
To: davejwatson
Cc: doronrk, alexei.starovoitov, john.fastabend, davem, netdev,
Daniel Borkmann
In kTLS MSG_PEEK behavior is currently failing, strace example:
[pid 2430] socket(AF_INET, SOCK_STREAM, IPPROTO_IP) = 3
[pid 2430] socket(AF_INET, SOCK_STREAM, IPPROTO_IP) = 4
[pid 2430] bind(4, {sa_family=AF_INET, sin_port=htons(0), sin_addr=inet_addr("0.0.0.0")}, 16) = 0
[pid 2430] listen(4, 10) = 0
[pid 2430] getsockname(4, {sa_family=AF_INET, sin_port=htons(38855), sin_addr=inet_addr("0.0.0.0")}, [16]) = 0
[pid 2430] connect(3, {sa_family=AF_INET, sin_port=htons(38855), sin_addr=inet_addr("0.0.0.0")}, 16) = 0
[pid 2430] setsockopt(3, SOL_TCP, 0x1f /* TCP_??? */, [7564404], 4) = 0
[pid 2430] setsockopt(3, 0x11a /* SOL_?? */, 1, "\3\0033\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 40) = 0
[pid 2430] accept(4, {sa_family=AF_INET, sin_port=htons(49636), sin_addr=inet_addr("127.0.0.1")}, [16]) = 5
[pid 2430] setsockopt(5, SOL_TCP, 0x1f /* TCP_??? */, [7564404], 4) = 0
[pid 2430] setsockopt(5, 0x11a /* SOL_?? */, 2, "\3\0033\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 40) = 0
[pid 2430] close(4) = 0
[pid 2430] sendto(3, "test_read_peek", 14, 0, NULL, 0) = 14
[pid 2430] sendto(3, "_mult_recs\0", 11, 0, NULL, 0) = 11
[pid 2430] recvfrom(5, "test_read_peektest_read_peektest"..., 64, MSG_PEEK, NULL, NULL) = 64
As can be seen from strace, there are two TLS records sent,
i) 'test_read_peek' and ii) '_mult_recs\0' where we end up
peeking 'test_read_peektest_read_peektest'. This is clearly
wrong, and what happens is that given peek cannot call into
tls_sw_advance_skb() to unpause strparser and proceed with
the next skb, we end up looping over the current one, copying
the 'test_read_peek' over and over into the user provided
buffer.
Here, we can only peek into the currently held skb (current,
full TLS record) as otherwise we would end up having to hold
all the original skb(s) (depending on the peek depth) in a
separate queue when unpausing strparser to process next
records, minimally intrusive is to return only up to the
current record's size (which likely was what c46234ebb4d1
("tls: RX path for ktls") originally intended as well). Thus,
after patch we properly peek the first record:
[pid 2046] wait4(2075, <unfinished ...>
[pid 2075] socket(AF_INET, SOCK_STREAM, IPPROTO_IP) = 3
[pid 2075] socket(AF_INET, SOCK_STREAM, IPPROTO_IP) = 4
[pid 2075] bind(4, {sa_family=AF_INET, sin_port=htons(0), sin_addr=inet_addr("0.0.0.0")}, 16) = 0
[pid 2075] listen(4, 10) = 0
[pid 2075] getsockname(4, {sa_family=AF_INET, sin_port=htons(55115), sin_addr=inet_addr("0.0.0.0")}, [16]) = 0
[pid 2075] connect(3, {sa_family=AF_INET, sin_port=htons(55115), sin_addr=inet_addr("0.0.0.0")}, 16) = 0
[pid 2075] setsockopt(3, SOL_TCP, 0x1f /* TCP_??? */, [7564404], 4) = 0
[pid 2075] setsockopt(3, 0x11a /* SOL_?? */, 1, "\3\0033\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 40) = 0
[pid 2075] accept(4, {sa_family=AF_INET, sin_port=htons(45732), sin_addr=inet_addr("127.0.0.1")}, [16]) = 5
[pid 2075] setsockopt(5, SOL_TCP, 0x1f /* TCP_??? */, [7564404], 4) = 0
[pid 2075] setsockopt(5, 0x11a /* SOL_?? */, 2, "\3\0033\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 40) = 0
[pid 2075] close(4) = 0
[pid 2075] sendto(3, "test_read_peek", 14, 0, NULL, 0) = 14
[pid 2075] sendto(3, "_mult_recs\0", 11, 0, NULL, 0) = 11
[pid 2075] recvfrom(5, "test_read_peek", 64, MSG_PEEK, NULL, NULL) = 14
Fixes: c46234ebb4d1 ("tls: RX path for ktls")
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
net/tls/tls_sw.c | 8 +++++++
tools/testing/selftests/net/tls.c | 49 +++++++++++++++++++++++++++++++++++++++
2 files changed, 57 insertions(+)
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index e28a6ff..b0cea79 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -931,7 +931,15 @@ int tls_sw_recvmsg(struct sock *sk,
if (control != TLS_RECORD_TYPE_DATA)
goto recv_end;
}
+ } else {
+ /* MSG_PEEK right now cannot look beyond current skb
+ * from strparser, meaning we cannot advance skb here
+ * and thus unpause strparser since we'd loose original
+ * one.
+ */
+ break;
}
+
/* If we have a new message from strparser, continue now. */
if (copied >= target && !ctx->recv_pkt)
break;
diff --git a/tools/testing/selftests/net/tls.c b/tools/testing/selftests/net/tls.c
index b3ebf26..8fdfeaf 100644
--- a/tools/testing/selftests/net/tls.c
+++ b/tools/testing/selftests/net/tls.c
@@ -502,6 +502,55 @@ TEST_F(tls, recv_peek_multiple)
EXPECT_EQ(memcmp(test_str, buf, send_len), 0);
}
+TEST_F(tls, recv_peek_multiple_records)
+{
+ char const *test_str = "test_read_peek_mult_recs";
+ char const *test_str_first = "test_read_peek";
+ char const *test_str_second = "_mult_recs";
+ int len;
+ char buf[64];
+
+ len = strlen(test_str_first);
+ EXPECT_EQ(send(self->fd, test_str_first, len, 0), len);
+
+ len = strlen(test_str_second) + 1;
+ EXPECT_EQ(send(self->fd, test_str_second, len, 0), len);
+
+ len = sizeof(buf);
+ memset(buf, 0, len);
+ EXPECT_NE(recv(self->cfd, buf, len, MSG_PEEK), -1);
+
+ /* MSG_PEEK can only peek into the current record. */
+ len = strlen(test_str_first) + 1;
+ EXPECT_EQ(memcmp(test_str_first, buf, len), 0);
+
+ len = sizeof(buf);
+ memset(buf, 0, len);
+ EXPECT_NE(recv(self->cfd, buf, len, 0), -1);
+
+ /* Non-MSG_PEEK will advance strparser (and therefore record)
+ * however.
+ */
+ len = strlen(test_str) + 1;
+ EXPECT_EQ(memcmp(test_str, buf, len), 0);
+
+ /* MSG_MORE will hold current record open, so later MSG_PEEK
+ * will see everything.
+ */
+ len = strlen(test_str_first);
+ EXPECT_EQ(send(self->fd, test_str_first, len, MSG_MORE), len);
+
+ len = strlen(test_str_second) + 1;
+ EXPECT_EQ(send(self->fd, test_str_second, len, 0), len);
+
+ len = sizeof(buf);
+ memset(buf, 0, len);
+ EXPECT_NE(recv(self->cfd, buf, len, MSG_PEEK), -1);
+
+ len = strlen(test_str) + 1;
+ EXPECT_EQ(memcmp(test_str, buf, len), 0);
+}
+
TEST_F(tls, pollin)
{
char const *test_str = "test_poll";
--
2.9.5
^ permalink raw reply related
* Re: [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg
From: Al Viro @ 2018-09-14 20:57 UTC (permalink / raw)
To: Darren Hart
Cc: Arnd Bergmann, linux-fsdevel, Greg Kroah-Hartman, David S. Miller,
devel, linux-kernel, qat-linux, linux-crypto, linux-media,
dri-devel, linaro-mm-sig, amd-gfx, linux-input, linux-iio,
linux-rdma, linux-nvdimm, linux-nvme, linux-pci,
platform-driver-x86, linux-remoteproc, sparclinux, linux-scsi,
linux-usb, linux-fbdev, linuxppc-dev, linux-btrfs
In-Reply-To: <20180914203506.GE35251@wrath>
On Fri, Sep 14, 2018 at 01:35:06PM -0700, Darren Hart wrote:
> Acked-by: Darren Hart (VMware) <dvhart@infradead.org>
>
> As for a longer term solution, would it be possible to init fops in such
> a way that the compat_ioctl call defaults to generic_compat_ioctl_ptrarg
> so we don't have to duplicate this boilerplate for every ioctl fops
> structure?
Bad idea, that... Because several years down the road somebody will add
an ioctl that takes an unsigned int for argument. Without so much as looking
at your magical mystery macro being used to initialize file_operations.
FWIW, I would name that helper in more blunt way - something like
compat_ioctl_only_compat_pointer_ioctls_here()...
^ permalink raw reply
* Re: [PATCH net-next v2] net: sched: change tcf_del_walker() to take idrinfo->lock
From: Cong Wang @ 2018-09-14 20:53 UTC (permalink / raw)
To: Vlad Buslov
Cc: Linux Kernel Network Developers, Jamal Hadi Salim, Jiri Pirko,
David Miller
In-Reply-To: <vbfmusknypr.fsf@reg-r-vrt-018-180.mtr.labs.mlnx>
On Fri, Sep 14, 2018 at 3:46 AM Vlad Buslov <vladbu@mellanox.com> wrote:
>
>
> On Thu 13 Sep 2018 at 17:13, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> > On Wed, Sep 12, 2018 at 1:51 AM Vlad Buslov <vladbu@mellanox.com> wrote:
> >>
> >>
> >> On Fri 07 Sep 2018 at 19:12, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> >> > On Fri, Sep 7, 2018 at 6:52 AM Vlad Buslov <vladbu@mellanox.com> wrote:
> >> >>
> >> >> Action API was changed to work with actions and action_idr in concurrency
> >> >> safe manner, however tcf_del_walker() still uses actions without taking a
> >> >> reference or idrinfo->lock first, and deletes them directly, disregarding
> >> >> possible concurrent delete.
> >> >>
> >> >> Add tc_action_wq workqueue to action API. Implement
> >> >> tcf_idr_release_unsafe() that assumes external synchronization by caller
> >> >> and delays blocking action cleanup part to tc_action_wq workqueue. Extend
> >> >> tcf_action_cleanup() with 'async' argument to indicate that function should
> >> >> free action asynchronously.
> >> >
> >> > Where exactly is blocking in tcf_action_cleanup()?
> >> >
> >> > From your code, it looks like free_tcf(), but from my observation,
> >> > the only blocking function inside is tcf_action_goto_chain_fini()
> >> > which calls __tcf_chain_put(). But, __tcf_chain_put() is blocking
> >> > _ONLY_ when tc_chain_notify() is called, for tc action it is never
> >> > called.
> >> >
> >> > So, what else is blocking?
> >>
> >> __tcf_chain_put() calls tc_chain_tmplt_del(), which calls
> >> ops->tmplt_destroy(). This last function uses hw offload API, which is
> >> blocking.
> >
> > Good to know.
> >
> > Can we just make ops->tmplt_destroy() to use workqueue?
> > Making tc action to workqueue seems overkill, for me.
>
> How about changing tcf_chain_put_by_act() to use tc_filter_wq, instead
> of directly calling __tcf_chain_put()? IMO it is a better solution
> because it benefits all classifiers, instead of requiring every
> classifier with templates support to implement non-blocking
> ops->tmplt_destroy().
My point is, there is only one filter implements ops->tmplt_destroy
so far, so there is no reason to just make all filters to adjusted
for this single one. Not to mention actions, actions are innocent
here.
^ permalink raw reply
* [Patch net-next] ipv4: initialize ra_mutex in inet_init_net()
From: Cong Wang @ 2018-09-14 20:32 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, Kirill Tkhai
ra_mutex is a IPv4 specific mutex, it is inside struct netns_ipv4,
but its initialization is in the generic netns code, setup_net().
Move it to IPv4 specific net init code, inet_init_net().
Fixes: d9ff3049739e ("net: Replace ip_ra_lock with per-net mutex")
Cc: Kirill Tkhai <ktkhai@virtuozzo.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
net/core/net_namespace.c | 1 -
net/ipv4/af_inet.c | 2 ++
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
index 670c84b1bfc2..b272ccfcbf63 100644
--- a/net/core/net_namespace.c
+++ b/net/core/net_namespace.c
@@ -308,7 +308,6 @@ static __net_init int setup_net(struct net *net, struct user_namespace *user_ns)
net->user_ns = user_ns;
idr_init(&net->netns_ids);
spin_lock_init(&net->nsid_lock);
- mutex_init(&net->ipv4.ra_mutex);
list_for_each_entry(ops, &pernet_list, list) {
error = ops_init(ops, net);
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 20fda8fb8ffd..57b7bffb93e5 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -1817,6 +1817,8 @@ static __net_init int inet_init_net(struct net *net)
net->ipv4.sysctl_igmp_llm_reports = 1;
net->ipv4.sysctl_igmp_qrv = 2;
+ mutex_init(&net->ipv4.ra_mutex);
+
return 0;
}
--
2.14.4
^ permalink raw reply related
* Re: [PATCH net] bnxt_en: Fix VF mac address regression.
From: Siwei Liu @ 2018-09-14 20:14 UTC (permalink / raw)
To: Michael Chan; +Cc: David Miller, Netdev, seth.forshee, si-wei liu
In-Reply-To: <1536954089-6061-1-git-send-email-michael.chan@broadcom.com>
Ack. Looks fine to me.
-Siwei
On Fri, Sep 14, 2018 at 12:41 PM, Michael Chan
<michael.chan@broadcom.com> wrote:
> The recent commit to always forward the VF MAC address to the PF for
> approval may not work if the PF driver or the firmware is older. This
> will cause the VF driver to fail during probe:
>
> bnxt_en 0000:00:03.0 (unnamed net_device) (uninitialized): hwrm req_type 0xf seq id 0x5 error 0xffff
> bnxt_en 0000:00:03.0 (unnamed net_device) (uninitialized): VF MAC address 00:00:17:02:05:d0 not approved by the PF
> bnxt_en 0000:00:03.0: Unable to initialize mac address.
> bnxt_en: probe of 0000:00:03.0 failed with error -99
>
> We fix it by treating the error as fatal only if the VF MAC address is
> locally generated by the VF.
>
> Fixes: 707e7e966026 ("bnxt_en: Always forward VF MAC address to the PF.")
> Reported-by: Seth Forshee <seth.forshee@canonical.com>
> Reported-by: Siwei Liu <loseweigh@gmail.com>
> Signed-off-by: Michael Chan <michael.chan@broadcom.com>
> ---
> Please queue this for stable as well. Thanks.
>
> drivers/net/ethernet/broadcom/bnxt/bnxt.c | 9 +++++++--
> drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c | 9 +++++----
> drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.h | 2 +-
> 3 files changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> index cecbb1d..177587f 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> @@ -8027,7 +8027,7 @@ static int bnxt_change_mac_addr(struct net_device *dev, void *p)
> if (ether_addr_equal(addr->sa_data, dev->dev_addr))
> return 0;
>
> - rc = bnxt_approve_mac(bp, addr->sa_data);
> + rc = bnxt_approve_mac(bp, addr->sa_data, true);
> if (rc)
> return rc;
>
> @@ -8827,14 +8827,19 @@ static int bnxt_init_mac_addr(struct bnxt *bp)
> } else {
> #ifdef CONFIG_BNXT_SRIOV
> struct bnxt_vf_info *vf = &bp->vf;
> + bool strict_approval = true;
>
> if (is_valid_ether_addr(vf->mac_addr)) {
> /* overwrite netdev dev_addr with admin VF MAC */
> memcpy(bp->dev->dev_addr, vf->mac_addr, ETH_ALEN);
> + /* Older PF driver or firmware may not approve this
> + * correctly.
> + */
> + strict_approval = false;
> } else {
> eth_hw_addr_random(bp->dev);
> }
> - rc = bnxt_approve_mac(bp, bp->dev->dev_addr);
> + rc = bnxt_approve_mac(bp, bp->dev->dev_addr, strict_approval);
> #endif
> }
> return rc;
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c
> index fcd085a..3962f6f 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c
> @@ -1104,7 +1104,7 @@ void bnxt_update_vf_mac(struct bnxt *bp)
> mutex_unlock(&bp->hwrm_cmd_lock);
> }
>
> -int bnxt_approve_mac(struct bnxt *bp, u8 *mac)
> +int bnxt_approve_mac(struct bnxt *bp, u8 *mac, bool strict)
> {
> struct hwrm_func_vf_cfg_input req = {0};
> int rc = 0;
> @@ -1122,12 +1122,13 @@ int bnxt_approve_mac(struct bnxt *bp, u8 *mac)
> memcpy(req.dflt_mac_addr, mac, ETH_ALEN);
> rc = hwrm_send_message(bp, &req, sizeof(req), HWRM_CMD_TIMEOUT);
> mac_done:
> - if (rc) {
> + if (rc && strict) {
> rc = -EADDRNOTAVAIL;
> netdev_warn(bp->dev, "VF MAC address %pM not approved by the PF\n",
> mac);
> + return rc;
> }
> - return rc;
> + return 0;
> }
> #else
>
> @@ -1144,7 +1145,7 @@ void bnxt_update_vf_mac(struct bnxt *bp)
> {
> }
>
> -int bnxt_approve_mac(struct bnxt *bp, u8 *mac)
> +int bnxt_approve_mac(struct bnxt *bp, u8 *mac, bool strict)
> {
> return 0;
> }
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.h
> index e9b20cd..2eed9ed 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.h
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.h
> @@ -39,5 +39,5 @@ int bnxt_sriov_configure(struct pci_dev *pdev, int num_vfs);
> void bnxt_sriov_disable(struct bnxt *);
> void bnxt_hwrm_exec_fwd_req(struct bnxt *);
> void bnxt_update_vf_mac(struct bnxt *);
> -int bnxt_approve_mac(struct bnxt *, u8 *);
> +int bnxt_approve_mac(struct bnxt *, u8 *, bool);
> #endif
> --
> 2.5.1
>
^ permalink raw reply
* Re: [PATCH v2,net-next 1/2] ip_gre: fix parsing gre header in ipgre_err
From: Haishuang Yan @ 2018-09-15 1:22 UTC (permalink / raw)
To: Edward Cree; +Cc: David Miller, kuznet, jbenc, netdev, linux-kernel
In-Reply-To: <4bd44714-8190-feca-27dc-6f6b254341f8@solarflare.com>
> On 2018年9月14日, at 下午8:44, Edward Cree <ecree@solarflare.com> wrote:
>
> On 13/09/18 18:58, David Miller wrote:
>> From: Haishuang Yan <yanhaishuang@cmss.chinamobile.com>
>> Date: Wed, 12 Sep 2018 17:21:21 +0800
>>
>>> @@ -86,7 +86,7 @@ int gre_parse_header(struct sk_buff *skb, struct tnl_ptk_info *tpi,
>>>
>>> options = (__be32 *)(greh + 1);
>>> if (greh->flags & GRE_CSUM) {
>>> - if (skb_checksum_simple_validate(skb)) {
>>> + if (csum_err && skb_checksum_simple_validate(skb)) {
>>> *csum_err = true;
>>> return -EINVAL;
>>> }
>> You want to ignore csum errors, but you do not want to elide the side
>> effects of the skb_checksum_simple_validate() call which are to set
>> skb->csum_valid and skb->csum.
>>
>> Therefore, the skb_checksum_simple_validate() call still needs to be
>> performed. We just wont return -EINVAL in the NULL csum_err case.
>
> How about just reversing the order of the AND?
>
> if (skb_checksum_simple_validate(skb) && csum_err) {
> *csum_err = true;
> return -EINVAL;
> }
>
>
It looks good to me, thanks!
But skb_checksum_try_convert only need to be called after the checksum is
validated, so I suggested a better solution as following:
89 if (!skb_checksum_simple_validate(skb)) {
90 skb_checksum_try_convert(skb, IPPROTO_GRE, 0,
91 null_compute_pseudo);
92 } else if (csum_err) {
93 *csum_err = true;
94 return -EINVAL;
95 }
^ permalink raw reply
* [net-next PATCH] tls: async support causes out-of-bounds access in crypto APIs
From: John Fastabend @ 2018-09-14 20:01 UTC (permalink / raw)
To: vakul.garg, davejwatson
Cc: doronrk, netdev, alexei.starovoitov, daniel, davem
When async support was added it needed to access the sk from the async
callback to report errors up the stack. The patch tried to use space
after the aead request struct by directly setting the reqsize field in
aead_request. This is an internal field that should not be used
outside the crypto APIs. It is used by the crypto code to define extra
space for private structures used in the crypto context. Users of the
API then use crypto_aead_reqsize() and add the returned amount of
bytes to the end of the request memory allocation before posting the
request to encrypt/decrypt APIs.
So this breaks (with general protection fault and KASAN error, if
enabled) because the request sent to decrypt is shorter than required
causing the crypto API out-of-bounds errors. Also it seems unlikely the
sk is even valid by the time it gets to the callback because of memset
in crypto layer.
Anyways, fix this by holding the sk in the skb->sk field when the
callback is set up and because the skb is already passed through to
the callback handler via void* we can access it in the handler. Then
in the handler we need to be careful to NULL the pointer again before
kfree_skb. I added comments on both the setup (in tls_do_decryption)
and when we clear it from the crypto callback handler
tls_decrypt_done(). After this selftests pass again and fixes KASAN
errors/warnings.
Fixes: 94524d8fc965 ("net/tls: Add support for async decryption of tls records")
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
include/net/tls.h | 4 ----
net/tls/tls_sw.c | 39 +++++++++++++++++++++++----------------
2 files changed, 23 insertions(+), 20 deletions(-)
diff --git a/include/net/tls.h b/include/net/tls.h
index cd0a65b..8630d28 100644
--- a/include/net/tls.h
+++ b/include/net/tls.h
@@ -128,10 +128,6 @@ struct tls_sw_context_rx {
bool async_notify;
};
-struct decrypt_req_ctx {
- struct sock *sk;
-};
-
struct tls_record_info {
struct list_head list;
u32 end_seq;
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index be4f2e9..cef69b6 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -122,25 +122,32 @@ static int skb_nsg(struct sk_buff *skb, int offset, int len)
static void tls_decrypt_done(struct crypto_async_request *req, int err)
{
struct aead_request *aead_req = (struct aead_request *)req;
- struct decrypt_req_ctx *req_ctx =
- (struct decrypt_req_ctx *)(aead_req + 1);
-
struct scatterlist *sgout = aead_req->dst;
-
- struct tls_context *tls_ctx = tls_get_ctx(req_ctx->sk);
- struct tls_sw_context_rx *ctx = tls_sw_ctx_rx(tls_ctx);
- int pending = atomic_dec_return(&ctx->decrypt_pending);
+ struct tls_sw_context_rx *ctx;
+ struct tls_context *tls_ctx;
struct scatterlist *sg;
+ struct sk_buff *skb;
unsigned int pages;
+ int pending;
+
+ skb = (struct sk_buff *)req->data;
+ tls_ctx = tls_get_ctx(skb->sk);
+ ctx = tls_sw_ctx_rx(tls_ctx);
+ pending = atomic_dec_return(&ctx->decrypt_pending);
/* Propagate if there was an err */
if (err) {
ctx->async_wait.err = err;
- tls_err_abort(req_ctx->sk, err);
+ tls_err_abort(skb->sk, err);
}
+ /* After using skb->sk to propagate sk through crypto async callback
+ * we need to NULL it again.
+ */
+ skb->sk = NULL;
+
/* Release the skb, pages and memory allocated for crypto req */
- kfree_skb(req->data);
+ kfree_skb(skb);
/* Skip the first S/G entry as it points to AAD */
for_each_sg(sg_next(sgout), sg, UINT_MAX, pages) {
@@ -175,11 +182,13 @@ static int tls_do_decryption(struct sock *sk,
(u8 *)iv_recv);
if (async) {
- struct decrypt_req_ctx *req_ctx;
-
- req_ctx = (struct decrypt_req_ctx *)(aead_req + 1);
- req_ctx->sk = sk;
-
+ /* Using skb->sk to push sk through to crypto async callback
+ * handler. This allows propagating errors up to the socket
+ * if needed. It _must_ be cleared in the async handler
+ * before kfree_skb is called. We _know_ skb->sk is NULL
+ * because it is a clone from strparser.
+ */
+ skb->sk = sk;
aead_request_set_callback(aead_req,
CRYPTO_TFM_REQ_MAY_BACKLOG,
tls_decrypt_done, skb);
@@ -1455,8 +1464,6 @@ int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx)
goto free_aead;
if (sw_ctx_rx) {
- (*aead)->reqsize = sizeof(struct decrypt_req_ctx);
-
/* Set up strparser */
memset(&cb, 0, sizeof(cb));
cb.rcv_msg = tls_queue;
^ permalink raw reply related
* [PATCH net] bnxt_en: Fix VF mac address regression.
From: Michael Chan @ 2018-09-14 19:41 UTC (permalink / raw)
To: davem; +Cc: netdev, seth.forshee, loseweigh
The recent commit to always forward the VF MAC address to the PF for
approval may not work if the PF driver or the firmware is older. This
will cause the VF driver to fail during probe:
bnxt_en 0000:00:03.0 (unnamed net_device) (uninitialized): hwrm req_type 0xf seq id 0x5 error 0xffff
bnxt_en 0000:00:03.0 (unnamed net_device) (uninitialized): VF MAC address 00:00:17:02:05:d0 not approved by the PF
bnxt_en 0000:00:03.0: Unable to initialize mac address.
bnxt_en: probe of 0000:00:03.0 failed with error -99
We fix it by treating the error as fatal only if the VF MAC address is
locally generated by the VF.
Fixes: 707e7e966026 ("bnxt_en: Always forward VF MAC address to the PF.")
Reported-by: Seth Forshee <seth.forshee@canonical.com>
Reported-by: Siwei Liu <loseweigh@gmail.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
Please queue this for stable as well. Thanks.
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 9 +++++++--
drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c | 9 +++++----
drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.h | 2 +-
3 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index cecbb1d..177587f 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -8027,7 +8027,7 @@ static int bnxt_change_mac_addr(struct net_device *dev, void *p)
if (ether_addr_equal(addr->sa_data, dev->dev_addr))
return 0;
- rc = bnxt_approve_mac(bp, addr->sa_data);
+ rc = bnxt_approve_mac(bp, addr->sa_data, true);
if (rc)
return rc;
@@ -8827,14 +8827,19 @@ static int bnxt_init_mac_addr(struct bnxt *bp)
} else {
#ifdef CONFIG_BNXT_SRIOV
struct bnxt_vf_info *vf = &bp->vf;
+ bool strict_approval = true;
if (is_valid_ether_addr(vf->mac_addr)) {
/* overwrite netdev dev_addr with admin VF MAC */
memcpy(bp->dev->dev_addr, vf->mac_addr, ETH_ALEN);
+ /* Older PF driver or firmware may not approve this
+ * correctly.
+ */
+ strict_approval = false;
} else {
eth_hw_addr_random(bp->dev);
}
- rc = bnxt_approve_mac(bp, bp->dev->dev_addr);
+ rc = bnxt_approve_mac(bp, bp->dev->dev_addr, strict_approval);
#endif
}
return rc;
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c
index fcd085a..3962f6f 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c
@@ -1104,7 +1104,7 @@ void bnxt_update_vf_mac(struct bnxt *bp)
mutex_unlock(&bp->hwrm_cmd_lock);
}
-int bnxt_approve_mac(struct bnxt *bp, u8 *mac)
+int bnxt_approve_mac(struct bnxt *bp, u8 *mac, bool strict)
{
struct hwrm_func_vf_cfg_input req = {0};
int rc = 0;
@@ -1122,12 +1122,13 @@ int bnxt_approve_mac(struct bnxt *bp, u8 *mac)
memcpy(req.dflt_mac_addr, mac, ETH_ALEN);
rc = hwrm_send_message(bp, &req, sizeof(req), HWRM_CMD_TIMEOUT);
mac_done:
- if (rc) {
+ if (rc && strict) {
rc = -EADDRNOTAVAIL;
netdev_warn(bp->dev, "VF MAC address %pM not approved by the PF\n",
mac);
+ return rc;
}
- return rc;
+ return 0;
}
#else
@@ -1144,7 +1145,7 @@ void bnxt_update_vf_mac(struct bnxt *bp)
{
}
-int bnxt_approve_mac(struct bnxt *bp, u8 *mac)
+int bnxt_approve_mac(struct bnxt *bp, u8 *mac, bool strict)
{
return 0;
}
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.h
index e9b20cd..2eed9ed 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.h
@@ -39,5 +39,5 @@ int bnxt_sriov_configure(struct pci_dev *pdev, int num_vfs);
void bnxt_sriov_disable(struct bnxt *);
void bnxt_hwrm_exec_fwd_req(struct bnxt *);
void bnxt_update_vf_mac(struct bnxt *);
-int bnxt_approve_mac(struct bnxt *, u8 *);
+int bnxt_approve_mac(struct bnxt *, u8 *, bool);
#endif
--
2.5.1
^ permalink raw reply related
* Re: [PATCH net-next v2] net/tls: Add support for async decryption of tls records
From: John Fastabend @ 2018-09-14 19:39 UTC (permalink / raw)
To: Vakul Garg, netdev; +Cc: borisp, aviadye, davejwatson, davem
In-Reply-To: <20180829095655.31963-1-vakul.garg@nxp.com>
On 08/29/2018 02:56 AM, Vakul Garg wrote:
> When tls records are decrypted using asynchronous acclerators such as
> NXP CAAM engine, the crypto apis return -EINPROGRESS. Presently, on
> getting -EINPROGRESS, the tls record processing stops till the time the
> crypto accelerator finishes off and returns the result. This incurs a
> context switch and is not an efficient way of accessing the crypto
> accelerators. Crypto accelerators work efficient when they are queued
> with multiple crypto jobs without having to wait for the previous ones
> to complete.
>
> The patch submits multiple crypto requests without having to wait for
> for previous ones to complete. This has been implemented for records
> which are decrypted in zero-copy mode. At the end of recvmsg(), we wait
> for all the asynchronous decryption requests to complete.
>
> The references to records which have been sent for async decryption are
> dropped. For cases where record decryption is not possible in zero-copy
> mode, asynchronous decryption is not used and we wait for decryption
> crypto api to complete.
>
> For crypto requests executing in async fashion, the memory for
> aead_request, sglists and skb etc is freed from the decryption
> completion handler. The decryption completion handler wakesup the
> sleeping user context when recvmsg() flags that it has done sending
> all the decryption requests and there are no more decryption requests
> pending to be completed.
>
> Signed-off-by: Vakul Garg <vakul.garg@nxp.com>
> Reviewed-by: Dave Watson <davejwatson@fb.com>
> ---
[...]
> @@ -1271,6 +1377,8 @@ int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx)
> goto free_aead;
>
> if (sw_ctx_rx) {
> + (*aead)->reqsize = sizeof(struct decrypt_req_ctx);
> +
This is not valid and may cause GPF or best case only a KASAN
warning. 'reqsize' should probably not be mangled outside the
internal crypto APIs but the real reason is the reqsize is used
to determine how much space is needed at the end of the aead_request
for crypto private ctx use in encrypt/decrypt. After this patch
when we submit an aead_request the crypto layer will think it
has room for its private structs at the end but now only 8B will
be there and crypto layer will happily memset some arbitrary
memory for you amongst other things.
Anyways testing a fix now will post shortly.
Thanks,
John
^ permalink raw reply
* Re: [bpf-next, v4 0/5] Introduce eBPF flow dissector
From: Alexei Starovoitov @ 2018-09-14 19:22 UTC (permalink / raw)
To: Petar Penkov
Cc: netdev, davem, ast, daniel, simon.horman, ecree, songliubraving,
tom, Petar Penkov
In-Reply-To: <20180914144622.16436-1-peterpenkov96@gmail.com>
On Fri, Sep 14, 2018 at 07:46:17AM -0700, Petar Penkov wrote:
> From: Petar Penkov <ppenkov@google.com>
>
> This patch series hardens the RX stack by allowing flow dissection in BPF,
> as previously discussed [1]. Because of the rigorous checks of the BPF
> verifier, this provides significant security guarantees. In particular, the
> BPF flow dissector cannot get inside of an infinite loop, as with
> CVE-2013-4348, because BPF programs are guaranteed to terminate. It cannot
> read outside of packet bounds, because all memory accesses are checked.
> Also, with BPF the administrator can decide which protocols to support,
> reducing potential attack surface. Rarely encountered protocols can be
> excluded from dissection and the program can be updated without kernel
> recompile or reboot if a bug is discovered.
>
> Patch 1 adds infrastructure to execute a BPF program in __skb_flow_dissect.
> This includes a new BPF program and attach type.
>
> Patch 2 adds the new BPF flow dissector definitions to tools/uapi.
>
> Patch 3 adds support for the new BPF program type to libbpf and bpftool.
>
> Patch 4 adds a flow dissector program in BPF. This parses most protocols in
> __skb_flow_dissect in BPF for a subset of flow keys (basic, control, ports,
> and address types).
>
> Patch 5 adds a selftest that attaches the BPF program to the flow dissector
> and sends traffic with different levels of encapsulation.
>
> Performance Evaluation:
> The in-kernel implementation was compared against the demo program from
> patch 4 using the test in patch 5 with IPv4/UDP traffic over 10 seconds.
> $perf record -a -C 4 taskset -c 4 ./test_flow_dissector -i 4 -f 8 \
> -t 10
Looks great. Applied to bpf-next with one extra patch:
SEC("dissect")
-int dissect(struct __sk_buff *skb)
+int _dissect(struct __sk_buff *skb)
otherwise the test doesn't build.
I'm not sure how it builds for you. Which llvm did you use?
Also above command works and ipv4 test in ./test_flow_dissector.sh
is passing as well, but it still fails at the end for me:
./test_flow_dissector.sh
bpffs not mounted. Mounting...
0: IP
1: IPV6
2: IPV6OP
3: IPV6FR
4: MPLS
5: VLAN
Testing IPv4...
inner.dest4: 127.0.0.1
inner.source4: 127.0.0.3
pkts: tx=10 rx=10
inner.dest4: 127.0.0.1
inner.source4: 127.0.0.3
pkts: tx=10 rx=0
inner.dest4: 127.0.0.1
inner.source4: 127.0.0.3
pkts: tx=10 rx=10
Testing IPIP...
tunnels before test:
tunl0: any/ip remote any local any ttl inherit nopmtudisc
sit_test_LV5N: any/ip remote 127.0.0.2 local 127.0.0.1 dev lo ttl inherit
ipip_test_LV5N: any/ip remote 127.0.0.2 local 127.0.0.1 dev lo ttl inherit
sit0: ipv6/ip remote any local any ttl 64 nopmtudisc
gre_test_LV5N: gre/ip remote 127.0.0.2 local 127.0.0.1 dev lo ttl inherit
gre0: gre/ip remote any local any ttl inherit nopmtudisc
inner.dest4: 192.168.0.1
inner.source4: 1.1.1.1
encap proto: 4
outer.dest4: 127.0.0.1
outer.source4: 127.0.0.2
pkts: tx=10 rx=0
tunnels after test:
tunl0: any/ip remote any local any ttl inherit nopmtudisc
sit0: ipv6/ip remote any local any ttl 64 nopmtudisc
gre0: gre/ip remote any local any ttl inherit nopmtudisc
selftests: test_flow_dissector [FAILED]
is it something in my setup or test is broken?
^ permalink raw reply
* Re: [RFC PATCH net-next v1 00/14] rename and shrink i40evf
From: Jesse Brandeburg @ 2018-09-14 19:17 UTC (permalink / raw)
To: Or Gerlitz
Cc: Linux Netdev List, intel-wired-lan, Jeff Kirsher, Saeed Mahameed
In-Reply-To: <CAJ3xEMj3OXwz=A5+5JQ0mXoq2OU3N7DP-TpJVMtdTWXZw5tQ9w@mail.gmail.com>
On Fri, 14 Sep 2018 12:10:45 +0300 Or wrote:
> On Fri, Sep 14, 2018 at 1:31 AM, Jesse Brandeburg
> <jesse.brandeburg@intel.com> wrote:
> on what HW ring format do you standardize? do i40e/Fortville and
> ice/what's-the-intel-code-name? HWs can/use the same posting/completion
> descriptor?
The initial ring format is the same as used for XL710/X722 devices, and
planned be supported for the Intel Ethernet E800 series (ice driver) and
future VF devices using SR-IOV.
> > This solves 2 issues we saw coming or were already present, the
> > first was constant code duplication happening with i40e/i40evf,
> > when much of the duplicate code in the i40evf was not used or was
> > not needed.
>
> could you spare few words on the origin/nature of these duplicates? were them
> just developer C&P mistakes for functionality which is irrelevant for
> a VF? like what?
> if not, what was there?
In particular, some of the code was not used at all, but was not caught
by any automation because it was in a header file and included into
multiple file scopes. Other big chunk of the duplicate code was for
the PF's usage of the communication channel to firmware, which for some
reason was left in the VF driver code (probably just to avoid changing
the file) - but the VF driver doesn't communicate to firmware, just to
the PF.
> > The second was to remove the future confusion of why
> > future VF devices that were not considered "40GbE" only devices
> > were supported by i40evf.
>
> can elaborate further?
The name i40evf was generating customer questions, and was confusing
when you add in multiple generations of PF hardware that are no longer
using the i40e driver.
> > The thought is that iavf will be the virtual function driver for
> > all future devices, so it should have a "generic" name to propery
> > represent that it is the VF driver for multiple generations of
> > devices.
>
> for that end, as I think was explained @ the netdev Tokyo AVF session,
> you would need a mechanism for feature negotiation, is it here or coming up?
The driver already has it (a feature negotitiation), please see the
function called iavf_send_vf_config_msg, and follow from where it is
called. Basically the VF driver negotiates with the PF for what it can
do, and the PF guarantees that the base set of features will always
work, with optional advanced features which the code may/may-not have
in the future.
> > 41 files changed, 3436 insertions(+), 7581 deletions(-)
>
> code diet is cool!
Thanks! ~4000 lines less made me very happy too.
^ 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