* Re: [PATCH net-next] net: check type when freeing metadata dst
From: Sergei Shtylyov @ 2017-08-18 12:25 UTC (permalink / raw)
To: David Lamparter, netdev
Cc: Jakub Kicinski, Sridhar Samudrala, Simon Horman, David S . Miller
In-Reply-To: <20170818121651.516877-1-equinox@diac24.net>
Hello!
On 08/18/2017 03:16 PM, David Lamparter wrote:
> There is a new metadata dst type field added in "net: store
> port/representator id in metadata_dst", but metadata_dst_free() wasn't
> updated to check it before freeing the METADATA_IP_TUNNEL specific dst
> cache entry.
>
> This is not currently causing problems since it's far enough back in the
> struct to be zeroed for the only other type currently in existance
> (METADATA_HW_PORT_MUX), but nevertheless it's not correct.
>
> Fixes: 3fcece12bc1b6dcdf0986f2cd9e8f63b1f9b6aa0
Please see Documentation/process/submitting-patches.rst on how this tag
(and the commit citing in your description as well) should look like.
> Signed-off-by: David Lamparter <equinox@diac24.net>
> Cc: Jakub Kicinski <jakub.kicinski@netronome.com>
> Cc: Sridhar Samudrala <sridhar.samudrala@intel.com>
> Cc: Simon Horman <horms@verge.net.au>
> Cc: David S. Miller <davem@davemloft.net>
[...]
MBR, Sergei
^ permalink raw reply
* [PATCH v3 4/4] dt-bindings: net: dwmac-sun8i: update documentation about integrated PHY
From: Corentin Labbe @ 2017-08-18 12:21 UTC (permalink / raw)
To: robh+dt, mark.rutland, linux, maxime.ripard, wens,
peppe.cavallaro, alexandre.torgue
Cc: devicetree, linux-arm-kernel, linux-kernel, netdev,
Corentin Labbe
In-Reply-To: <20170818122118.4925-1-clabbe.montjoie@gmail.com>
This patch add documentation about the MDIO switch used on sun8i-h3-emac
for integrated PHY.
Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
.../devicetree/bindings/net/dwmac-sun8i.txt | 112 +++++++++++++++++++--
1 file changed, 105 insertions(+), 7 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
index 725f3b187886..631084122532 100644
--- a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
+++ b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
@@ -39,7 +39,7 @@ Optional properties for the following compatibles:
- allwinner,leds-active-low: EPHY LEDs are active low
Required child node of emac:
-- mdio bus node: should be named mdio
+- mdio bus node: should be named mdio (or mdio_parent in case of H3)
Required properties of the mdio node:
- #address-cells: shall be 1
@@ -48,14 +48,21 @@ Required properties of the mdio node:
The device node referenced by "phy" or "phy-handle" should be a child node
of the mdio node. See phy.txt for the generic PHY bindings.
-Required properties of the phy node with the following compatibles:
+Required mdio-mux nodes for the following compatibles:
+ - "allwinner,sun8i-h3-emac",
+- a mdio-mux node with compatible = "allwinner,sun8i-h3-mdio-switch"
+ - mdio-parent-bus: The parent bus is "mdio_parent"
+ - two child mdio, one for the integrated mdio, one for the external mdio
+
+Required properties of the integrated phy node with the following compatibles:
- "allwinner,sun8i-h3-emac",
- "allwinner,sun8i-v3s-emac":
- clocks: a phandle to the reference clock for the EPHY
- resets: a phandle to the reset control for the EPHY
+- phy-is-integrated
+- Should be a child of the integrated mdio
-Example:
-
+Example with integrated PHY:
emac: ethernet@1c0b000 {
compatible = "allwinner,sun8i-h3-emac";
syscon = <&syscon>;
@@ -75,10 +82,101 @@ emac: ethernet@1c0b000 {
mdio: mdio {
#address-cells = <1>;
#size-cells = <0>;
- int_mii_phy: ethernet-phy@1 {
+ };
+ mdio-mux {
+ compatible = "allwinner,sun8i-h3-mdio-switch";
+ mdio-parent-bus = <&mdio>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ int_mdio: mdio@1 {
+ reg = <1>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ int_mii_phy: ethernet-phy@1 {
+ reg = <1>;
+ clocks = <&ccu CLK_BUS_EPHY>;
+ resets = <&ccu RST_BUS_EPHY>;
+ phy-is-integrated
+ };
+ };
+ ext_mdio: mdio@0 {
+ reg = <0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+};
+
+Example with external PHY:
+emac: ethernet@1c0b000 {
+ compatible = "allwinner,sun8i-h3-emac";
+ syscon = <&syscon>;
+ reg = <0x01c0b000 0x104>;
+ interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "macirq";
+ resets = <&ccu RST_BUS_EMAC>;
+ reset-names = "stmmaceth";
+ clocks = <&ccu CLK_BUS_EMAC>;
+ clock-names = "stmmaceth";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ phy-handle = <&ext_rgmii_phy>;
+ phy-mode = "rgmii";
+ allwinner,leds-active-low;
+ mdio: mdio {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+ mdio-mux {
+ compatible = "allwinner,sun8i-h3-mdio-switch";
+ mdio-parent-bus = <&mdio>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ int_mdio: mdio@1 {
+ reg = <1>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ int_mii_phy: ethernet-phy@1 {
+ reg = <1>;
+ clocks = <&ccu CLK_BUS_EPHY>;
+ resets = <&ccu RST_BUS_EPHY>;
+ phy-is-integrated
+ };
+ };
+ ext_mdio: mdio@0 {
+ reg = <0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ ext_rgmii_phy: ethernet-phy@1 {
+ reg = <1>;
+ };
+ };
+};
+
+Example with SoC without integrated PHY
+
+emac: ethernet@1c0b000 {
+ compatible = "allwinner,sun8i-a83t-emac";
+ syscon = <&syscon>;
+ reg = <0x01c0b000 0x104>;
+ interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "macirq";
+ resets = <&ccu RST_BUS_EMAC>;
+ reset-names = "stmmaceth";
+ clocks = <&ccu CLK_BUS_EMAC>;
+ clock-names = "stmmaceth";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ phy-handle = <&ext_rgmii_phy>;
+ phy-mode = "rgmii";
+ mdio: mdio {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ ext_rgmii_phy: ethernet-phy@1 {
reg = <1>;
- clocks = <&ccu CLK_BUS_EPHY>;
- resets = <&ccu RST_BUS_EPHY>;
};
};
};
--
2.13.0
^ permalink raw reply related
* [PATCH v3 3/4] net: stmmac: register parent MDIO node for sun8i-h3-emac
From: Corentin Labbe @ 2017-08-18 12:21 UTC (permalink / raw)
To: robh+dt, mark.rutland, linux, maxime.ripard, wens,
peppe.cavallaro, alexandre.torgue
Cc: devicetree, linux-arm-kernel, linux-kernel, netdev,
Corentin Labbe
In-Reply-To: <20170818122118.4925-1-clabbe.montjoie@gmail.com>
In case of a MDIO switch, the registered MDIO node should be
the parent of the PHY. Otherwise of_phy_connect will fail.
Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index a366b3747eeb..ca3cc99d8960 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -312,10 +312,12 @@ static int stmmac_dt_phy(struct plat_stmmacenet_data *plat,
static const struct of_device_id need_mdio_ids[] = {
{ .compatible = "snps,dwc-qos-ethernet-4.10" },
{ .compatible = "allwinner,sun8i-a83t-emac" },
- { .compatible = "allwinner,sun8i-h3-emac" },
{ .compatible = "allwinner,sun8i-v3s-emac" },
{ .compatible = "allwinner,sun50i-a64-emac" },
};
+ static const struct of_device_id need_mdio_mux_ids[] = {
+ { .compatible = "allwinner,sun8i-h3-emac" },
+ };
/* If phy-handle property is passed from DT, use it as the PHY */
plat->phy_node = of_parse_phandle(np, "phy-handle", 0);
@@ -332,7 +334,13 @@ static int stmmac_dt_phy(struct plat_stmmacenet_data *plat,
mdio = false;
}
- if (of_match_node(need_mdio_ids, np)) {
+ /*
+ * In case of a MDIO switch/mux, the registered MDIO node should be
+ * the parent of the PHY. Otherwise of_phy_connect will fail.
+ */
+ if (of_match_node(need_mdio_mux_ids, np)) {
+ plat->mdio_node = of_get_parent(plat->phy_node);
+ } else if (of_match_node(need_mdio_ids, np)) {
plat->mdio_node = of_get_child_by_name(np, "mdio");
} else {
/**
--
2.13.0
^ permalink raw reply related
* [PATCH v3 2/4] net: stmmac: dwmac-sun8i: choose internal PHY via phy-is-integrated
From: Corentin Labbe @ 2017-08-18 12:21 UTC (permalink / raw)
To: robh+dt, mark.rutland, linux, maxime.ripard, wens,
peppe.cavallaro, alexandre.torgue
Cc: devicetree, linux-arm-kernel, linux-kernel, netdev,
Corentin Labbe
In-Reply-To: <20170818122118.4925-1-clabbe.montjoie@gmail.com>
The current way to find if the phy is internal is to compare DT phy-mode
and emac_variant/internal_phy.
But it will negate a possible future SoC where an external PHY use the
same phy mode than the internal one.
This patch adds a new way to find if the PHY is internal, via
the phy-is-integrated property.
Since the internal_phy variable does not need anymore to contain the xMII mode
used by the internal PHY, it is still used for knowing the presence of an
internal PHY, so it is modified to a boolean soc_has_internal_phy.
Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index fffd6d5fc907..672553b652bd 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -41,14 +41,14 @@
* This value is used for disabling properly EMAC
* and used as a good starting value in case of the
* boot process(uboot) leave some stuff.
- * @internal_phy: Does the MAC embed an internal PHY
+ * @soc_has_internal_phy: Does the MAC embed an internal PHY
* @support_mii: Does the MAC handle MII
* @support_rmii: Does the MAC handle RMII
* @support_rgmii: Does the MAC handle RGMII
*/
struct emac_variant {
u32 default_syscon_value;
- int internal_phy;
+ bool soc_has_internal_phy;
bool support_mii;
bool support_rmii;
bool support_rgmii;
@@ -75,7 +75,7 @@ struct sunxi_priv_data {
static const struct emac_variant emac_variant_h3 = {
.default_syscon_value = 0x58000,
- .internal_phy = PHY_INTERFACE_MODE_MII,
+ .soc_has_internal_phy = true,
.support_mii = true,
.support_rmii = true,
.support_rgmii = true
@@ -83,20 +83,20 @@ static const struct emac_variant emac_variant_h3 = {
static const struct emac_variant emac_variant_v3s = {
.default_syscon_value = 0x38000,
- .internal_phy = PHY_INTERFACE_MODE_MII,
+ .soc_has_internal_phy = true,
.support_mii = true
};
static const struct emac_variant emac_variant_a83t = {
.default_syscon_value = 0,
- .internal_phy = 0,
+ .soc_has_internal_phy = false,
.support_mii = true,
.support_rgmii = true
};
static const struct emac_variant emac_variant_a64 = {
.default_syscon_value = 0,
- .internal_phy = 0,
+ .soc_has_internal_phy = false,
.support_mii = true,
.support_rmii = true,
.support_rgmii = true
@@ -648,7 +648,7 @@ static int sun8i_dwmac_set_syscon(struct stmmac_priv *priv)
"Current syscon value is not the default %x (expect %x)\n",
val, reg);
- if (gmac->variant->internal_phy) {
+ if (gmac->variant->soc_has_internal_phy) {
if (!gmac->use_internal_phy) {
/* switch to external PHY interface */
reg &= ~H3_EPHY_SELECT;
@@ -932,7 +932,7 @@ static int sun8i_dwmac_probe(struct platform_device *pdev)
}
plat_dat->interface = of_get_phy_mode(dev->of_node);
- if (plat_dat->interface == gmac->variant->internal_phy) {
+ if (of_property_read_bool(plat_dat->phy_node, "phy-is-integrated")) {
dev_info(&pdev->dev, "Will use internal PHY\n");
gmac->use_internal_phy = true;
gmac->ephy_clk = of_clk_get(plat_dat->phy_node, 0);
--
2.13.0
^ permalink raw reply related
* [PATCH v3 1/4] ARM: dts: sunxi: h3/h5: represent the mdio switch used by sun8i-h3-emac
From: Corentin Labbe @ 2017-08-18 12:21 UTC (permalink / raw)
To: robh+dt, mark.rutland, linux, maxime.ripard, wens,
peppe.cavallaro, alexandre.torgue
Cc: devicetree, linux-arm-kernel, linux-kernel, netdev,
Corentin Labbe
In-Reply-To: <20170818122118.4925-1-clabbe.montjoie@gmail.com>
Since dwmac-sun8i could use either an integrated PHY or an external PHY
(which could be at same MDIO address), we need to represent this selection
by a MDIO switch.
Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
arch/arm/boot/dts/sunxi-h3-h5.dtsi | 28 +++++++++++++++++++++++-----
1 file changed, 23 insertions(+), 5 deletions(-)
diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
index 4b599b5d26f6..74fdfcc9dfd3 100644
--- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
+++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
@@ -417,14 +417,32 @@
#size-cells = <0>;
status = "disabled";
- mdio: mdio {
+ mdio_parent: mdio {
#address-cells = <1>;
#size-cells = <0>;
- int_mii_phy: ethernet-phy@1 {
- compatible = "ethernet-phy-ieee802.3-c22";
+ };
+ mdio-mux {
+ compatible = "allwinner,sun8i-h3-mdio-switch";
+ mdio-parent-bus = <&mdio_parent>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ internal_mdio: mdio@1 {
reg = <1>;
- clocks = <&ccu CLK_BUS_EPHY>;
- resets = <&ccu RST_BUS_EPHY>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ int_mii_phy: ethernet-phy@1 {
+ compatible = "ethernet-phy-ieee802.3-c22";
+ reg = <1>;
+ clocks = <&ccu CLK_BUS_EPHY>;
+ resets = <&ccu RST_BUS_EPHY>;
+ phy-is-integrated;
+ };
+ };
+ mdio: mdio@0 {
+ reg = <0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
};
};
};
--
2.13.0
^ permalink raw reply related
* [PATCH v3 0/4] net: stmmac: Detect PHY location with phy-is-integrated
From: Corentin Labbe @ 2017-08-18 12:21 UTC (permalink / raw)
To: robh+dt, mark.rutland, linux, maxime.ripard, wens,
peppe.cavallaro, alexandre.torgue
Cc: devicetree, linux-arm-kernel, linux-kernel, netdev,
Corentin Labbe
Hello
The current way to find if the PHY is internal is to compare DT phy-mode
and emac_variant/internal_phy.
But it will negate a possible future SoC where an external PHY use the
same phy mode than the integrated one.
This patchs series adds a new way to find if the PHY is integrated, via
the phy-is-integrated DT property.
Since it exists both integrated and external ethernet-phy@1, they are merged in
the final DTB and so share all properties.
For avoiding this, and better represent the reality, we use a MDIO mux.
Note that sun8i-v3s-emac have also an integrated PHY, but since it lacks
any external PHY support, it is not necessary to add MDIO mux to it.
The first patch should go via the sunxi tree.
Note that this serie will need backporting the patch
"Documentation: net: phy: Add phy-is-integrated binding" which is in net-next
Thanks
Regards
Changes since v2:
- Add a MDIO mux for creating distinction between integrated and external MDIO.
- phy-is-integrated is not set in dtsi.
Changes since v1:
- Dropped phy-is-integrated documentation patch since another same patch was already merged
- Moved phy-is-integrated from SoC dtsi to final board DT.
Corentin Labbe (4):
ARM: dts: sunxi: h3/h5: represent the mdio switch used by
sun8i-h3-emac
net: stmmac: dwmac-sun8i: choose internal PHY via phy-is-integrated
net: stmmac: register parent MDIO node for sun8i-h3-emac
dt-bindings: net: dwmac-sun8i: update documentation about integrated
PHY
.../devicetree/bindings/net/dwmac-sun8i.txt | 112 +++++++++++++++++++--
arch/arm/boot/dts/sunxi-h3-h5.dtsi | 28 +++++-
drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 16 +--
.../net/ethernet/stmicro/stmmac/stmmac_platform.c | 12 ++-
4 files changed, 146 insertions(+), 22 deletions(-)
--
2.13.0
^ permalink raw reply
* [PATCH net-next] net: check type when freeing metadata dst
From: David Lamparter @ 2017-08-18 12:16 UTC (permalink / raw)
To: netdev
Cc: David Lamparter, Jakub Kicinski, Sridhar Samudrala, Simon Horman,
David S . Miller
There is a new metadata dst type field added in "net: store
port/representator id in metadata_dst", but metadata_dst_free() wasn't
updated to check it before freeing the METADATA_IP_TUNNEL specific dst
cache entry.
This is not currently causing problems since it's far enough back in the
struct to be zeroed for the only other type currently in existance
(METADATA_HW_PORT_MUX), but nevertheless it's not correct.
Fixes: 3fcece12bc1b6dcdf0986f2cd9e8f63b1f9b6aa0
Signed-off-by: David Lamparter <equinox@diac24.net>
Cc: Jakub Kicinski <jakub.kicinski@netronome.com>
Cc: Sridhar Samudrala <sridhar.samudrala@intel.com>
Cc: Simon Horman <horms@verge.net.au>
Cc: David S. Miller <davem@davemloft.net>
---
net/core/dst.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/core/dst.c b/net/core/dst.c
index 00aa972ad1a1..7954d1710d97 100644
--- a/net/core/dst.c
+++ b/net/core/dst.c
@@ -299,7 +299,8 @@ EXPORT_SYMBOL_GPL(metadata_dst_alloc);
void metadata_dst_free(struct metadata_dst *md_dst)
{
#ifdef CONFIG_DST_CACHE
- dst_cache_destroy(&md_dst->u.tun_info.dst_cache);
+ if (md_dst->type == METADATA_IP_TUNNEL)
+ dst_cache_destroy(&md_dst->u.tun_info.dst_cache);
#endif
kfree(md_dst);
}
--
2.13.0
^ permalink raw reply related
* Re: [PATCH net-next] tcp: export drops counter to /proc/net/tcp{,6}
From: Eric Dumazet @ 2017-08-18 12:14 UTC (permalink / raw)
To: Stéphan Gorget
Cc: netdev, Jeethu Rao, David S . Miller, Alexei Starovoitov,
Eric Dumazet, kernel-team, linux-kernel
In-Reply-To: <20170818102135.266832-1-sgorget@fb.com>
On Fri, 2017-08-18 at 03:21 -0700, Stéphan Gorget wrote:
> Those counters are exported for raw and udp but not for tcp, though they
> are incremented.
>
> An example where it is useful is chasing listen overflow. Listen overflow
> are counted as a global counter in LINUX_MIB_LISTENOVERFLOWS accessible
> in /proc/net/netstat but there is no way to find related drops in the
> information exported for tcp. With this patch it will make possible to
> correlate growth of LINUX_MIB_LISTENOVERFLOWS with growth of drops for
> a tcp socket.
Hi Stéphan.
Eons ago, the decision was taken to declare /proc interface frozen.
In linux-4.7 I provided all the information you need using the modern
inet_diag interface, [1]
Simply use iproute2/ss tool to access this information in a very
efficient way (like filtering done in the kernel, instead having to
parse a gigantic /proc output)
lpaa5:~# ss -tm state listening src :22
Recv-Q Send-Q Local Address:Port Peer Address:Port
0 128 *:ssh *:*
skmem:(r0,rb8388608,t0,tb8388608,f0,w0,o0,bl0,d7)
0 128 :::ssh :::*
skmem:(r0,rb8388608,t0,tb8388608,f0,w0,o0,bl0,d0)
You can see here that the IPv4 listener for ssh had 7 drops.
Thanks.
[1] list of relevant commits.
commit 9caad864151e525929d323de96cad382da49c3b2
Author: Eric Dumazet <edumazet@google.com>
Date: Fri Apr 1 08:52:20 2016 -0700
tcp: increment sk_drops for listeners
Goal: packets dropped by a listener are accounted for.
This adds tcp_listendrop() helper, and clears sk_drops in sk_clone_lock()
so that children do not inherit their parent drop count.
Note that we no longer increment LINUX_MIB_LISTENDROPS counter when
sending a SYNCOOKIE, since the SYN packet generated a SYNACK.
We already have a separate LINUX_MIB_SYNCOOKIESSENT
Signed-off-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
commit 532182cd610782db8c18230c2747626562032205
Author: Eric Dumazet <edumazet@google.com>
Date: Fri Apr 1 08:52:19 2016 -0700
tcp: increment sk_drops for dropped rx packets
Now ss can report sk_drops, we can instruct TCP to increment
this per socket counter when it drops an incoming frame, to refine
monitoring and debugging.
Following patch takes care of listeners drops.
Signed-off-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
commit 15239302edd46b184e758048253541fb211e315e
Author: Eric Dumazet <edumazet@google.com>
Date: Fri Apr 1 08:52:18 2016 -0700
sock_diag: add SK_MEMINFO_DROPS
Reporting sk_drops to user space was available for UDP
sockets using /proc interface.
Add this to sock_diag, so that we can have the same information
available to ss users, and we'll be able to add sk_drops
indications for TCP sockets as well.
Signed-off-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
^ permalink raw reply
* Re: Something hitting my total number of connections to the server
From: Eric Dumazet @ 2017-08-18 12:06 UTC (permalink / raw)
To: Akshat Kakkar; +Cc: netdev
In-Reply-To: <CAA5aLPjs2S0cV7MoYLB6MvZVKXomUmwpi=EVHDXh=AwR_xBqsw@mail.gmail.com>
On Fri, 2017-08-18 at 14:44 +0530, Akshat Kakkar wrote:
> On Thu, Aug 17, 2017 at 5:06 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > On Thu, 2017-08-17 at 14:35 +0530, Akshat Kakkar wrote:
> >
> >> I upgraded to 4.4 but still experiencing same issue.
> >> Please help.
> >
> > Still too old kernel, shoot again ;)
> >
> >
>
>
> Sorry but that's the maximum I can try as of now as its the LT version.
>
> Besides, this issue was not present in 2.6.32 but came with 3.10 and
> still there in 4.4, so I doubt if it has to do with some kernel and/or
> kernel parameters much as you guys are good enough not to keep an
> issue for so long (around 3 years).
>
> So please help.
netdev is the developer list.
We deal with recent kernels only. Because we already spent time fixing
all these issues, we are not going to spend time fixing old kernels.
Please to your distro provider to backport the needed patches.
^ permalink raw reply
* Re: [iproute PATCH v2 1/7] ipaddress: Make buffer for filter.flushb static
From: Phil Sutter @ 2017-08-18 11:57 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170817170931.24302-2-phil@nwl.cc>
On Thu, Aug 17, 2017 at 07:09:25PM +0200, Phil Sutter wrote:
> The buffer is accessed outside of the function defining it, so make it
> static.
>
> Signed-off-by: Phil Sutter <phil@nwl.cc>
Self-NACK: Access to flushb should be sane since all accessors are
called from ipaddr_flush(). Looking at the covscan output again, it
merely complained about filter.flusb still pointing to the local buffer
when ipaddr_flush() returns.
Cheers, Phil
^ permalink raw reply
* [PATCH net-next] net: hns3: Add support to change MTU in hardware & netdev
From: Salil Mehta @ 2017-08-18 11:35 UTC (permalink / raw)
To: davem-fT/PcQaiUtIeIZ0/mPfg9Q
Cc: salil.mehta-hv44wF8Li93QT0dZR+AlfA,
yisen.zhuang-hv44wF8Li93QT0dZR+AlfA,
lipeng321-hv44wF8Li93QT0dZR+AlfA,
dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA,
mehta.salil.lnk-Re5JQEeQqe8AvxtiuMwx3w,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linuxarm-hv44wF8Li93QT0dZR+AlfA
This patch adds the following support to the HNS3 driver:
1. Support to change the Maximum Transmission Unit of a
netdevice and of a port in hardware .
2. Initializes the supported MTU range for the netdevice.
Signed-off-by: lipeng <lipeng321-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
Signed-off-by: Salil Mehta <salil.mehta-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
---
.../net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.c | 46 ++++++++++++++++++++++
.../net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.h | 1 +
2 files changed, 47 insertions(+)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.c
index e731f87..8e3711e 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.c
@@ -1278,11 +1278,53 @@ static int hns3_ndo_set_vf_vlan(struct net_device *netdev, int vf, u16 vlan,
return ret;
}
+static int hns3_nic_change_mtu(struct net_device *netdev, int new_mtu)
+{
+ struct hns3_nic_priv *priv = netdev_priv(netdev);
+ struct hnae3_handle *h = priv->ae_handle;
+ bool if_running = netif_running(netdev);
+ int ret;
+
+ /* no change in MTU */
+ if (new_mtu == netdev->mtu)
+ return 0;
+
+ if (!h->ae_algo->ops->set_mtu)
+ return -ENOTSUPP;
+
+ /* if this was called with netdev up then bring netdevice down */
+ if (if_running) {
+ (void)hns3_nic_net_stop(netdev);
+ msleep(100);
+ }
+
+ ret = h->ae_algo->ops->set_mtu(h, new_mtu);
+ if (ret) {
+ netdev_err(netdev, "failed to change MTU in hardware %d\n",
+ ret);
+ return ret;
+ }
+
+ /* assign newly changed mtu to netdevice as well */
+ netdev->mtu = new_mtu;
+
+ /* if the netdev was running earlier, bring it up again */
+ if (if_running) {
+ if (hns3_nic_net_open(netdev)) {
+ netdev_err(netdev, "MTU, couldnt up netdev again\n");
+ ret = -EINVAL;
+ }
+ }
+
+ return ret;
+}
+
static const struct net_device_ops hns3_nic_netdev_ops = {
.ndo_open = hns3_nic_net_open,
.ndo_stop = hns3_nic_net_stop,
.ndo_start_xmit = hns3_nic_net_xmit,
.ndo_set_mac_address = hns3_nic_net_set_mac_address,
+ .ndo_change_mtu = hns3_nic_change_mtu,
.ndo_set_features = hns3_nic_set_features,
.ndo_get_stats64 = hns3_nic_get_stats64,
.ndo_setup_tc = hns3_nic_setup_tc,
@@ -2752,6 +2794,10 @@ static int hns3_client_init(struct hnae3_handle *handle)
goto out_reg_netdev_fail;
}
+ /* MTU range: 68 - 9706 */
+ netdev->min_mtu = ETH_MIN_MTU;
+ netdev->max_mtu = HNS3_MAX_MTU - (ETH_HLEN + ETH_FCS_LEN + VLAN_HLEN);
+
return ret;
out_reg_netdev_fail:
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.h b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.h
index a6e8f15..7e87461 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.h
@@ -76,6 +76,7 @@ enum hns3_nic_state {
#define HNS3_RING_NAME_LEN 16
#define HNS3_BUFFER_SIZE_2048 2048
#define HNS3_RING_MAX_PENDING 32768
+#define HNS3_MAX_MTU 9728
#define HNS3_BD_SIZE_512_TYPE 0
#define HNS3_BD_SIZE_1024_TYPE 1
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [net-next PATCH] ipv6: fix false-postive maybe-uninitialized warning
From: Arnd Bergmann @ 2017-08-18 11:34 UTC (permalink / raw)
To: David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI
Cc: Florian Westphal, Arnd Bergmann, David Ahern, Martin KaFai Lau,
Wei Wang, WANG Cong, netdev, linux-kernel
Adding a lock around one of the assignments prevents gcc from
tracking the state of the local 'fibmatch' variable, so it can no
longer prove that 'dst' is always initialized, leading to a bogus
warning:
net/ipv6/route.c: In function 'inet6_rtm_getroute':
net/ipv6/route.c:3659:2: error: 'dst' may be used uninitialized in this function [-Werror=maybe-uninitialized]
This moves the other assignment into the same lock to shut up the
warning.
Fixes: 121622dba8da ("ipv6: route: make rtm_getroute not assume rtnl is locked")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
net/ipv6/route.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
This kind of warning involving an unlock between variable initialization
and use is relatively frequent for false-positives. I should try to
seek clarification from the gcc developers on whether this can be
improved.
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index dc021ed6dd37..bec12ae3e6b7 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -3624,6 +3624,8 @@ static int inet6_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr *nlh,
if (!fibmatch)
dst = ip6_route_input_lookup(net, dev, &fl6, flags);
+ else
+ dst = ip6_route_lookup(net, &fl6, 0);
rcu_read_unlock();
} else {
@@ -3631,10 +3633,10 @@ static int inet6_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr *nlh,
if (!fibmatch)
dst = ip6_route_output(net, NULL, &fl6);
+ else
+ dst = ip6_route_lookup(net, &fl6, 0);
}
- if (fibmatch)
- dst = ip6_route_lookup(net, &fl6, 0);
rt = container_of(dst, struct rt6_info, dst);
if (rt->dst.error) {
--
2.9.0
^ permalink raw reply related
* Re: [PATCH] net: sunrpc: svcsock: fix NULL-pointer exception
From: Vadim Lomovtsev @ 2017-08-18 11:34 UTC (permalink / raw)
To: Jeff Layton
Cc: trond.myklebust-7I+n7zu2hftEKMMhf/gKZA,
anna.schumaker-HgOvQuBEEgTQT0dZR+AlfA,
bfields-uC3wQj2KruNg9hUCZPvPmw, davem-fT/PcQaiUtIeIZ0/mPfg9Q,
linux-nfs-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
pabeni-H+wXaHxf7aLQT0dZR+AlfA, vlomovts-H+wXaHxf7aLQT0dZR+AlfA
In-Reply-To: <1503055005.4719.17.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On Fri, Aug 18, 2017 at 07:16:45AM -0400, Jeff Layton wrote:
> On Fri, 2017-08-18 at 07:08 -0400, Vadim Lomovtsev wrote:
> > Hi Jeff,
> >
> > On Fri, Aug 18, 2017 at 06:27:32AM -0400, Jeff Layton wrote:
> > > On Fri, 2017-08-18 at 06:00 -0400, Vadim Lomovtsev wrote:
> > > > While running nfs/connectathon tests kernel NULL-pointer exception
> > > > has been observed due to races in svcsock.c.
> > > >
> > > > Race is appear when kernel accepts connection by kernel_accept
> > > > (which creates new socket) and start queuing ingress packets
> > > > to new socket. This happanes in ksoftirq context which concurrently
> > > > on a differnt core while new socket setup is not done yet.
> > > >
> > > > The fix is to re-order socket user data init sequence, add NULL-ptr
> > > > check before callback call along with barriers to prevent kernel crash.
> > > >
> > > > Test results: nfs/connectathon reports '0' failed tests for about 200+ iterations.
> > > >
> > > > Crash log:
> > > > ---<-snip->---
> > > > [ 6708.638984] Unable to handle kernel NULL pointer dereference at virtual address 00000000
> > > > [ 6708.647093] pgd = ffff0000094e0000
> > > > [ 6708.650497] [00000000] *pgd=0000010ffff90003, *pud=0000010ffff90003, *pmd=0000010ffff80003, *pte=0000000000000000
> > > > [ 6708.660761] Internal error: Oops: 86000005 [#1] SMP
> > > > [ 6708.665630] Modules linked in: nfsv3 nfnetlink_queue nfnetlink_log nfnetlink rpcsec_gss_krb5 nfsv4 dns_resolver nfs fscache overlay xt_CONNSECMARK xt_SECMARK xt_conntrack iptable_security ip_tables ah4 xfrm4_mode_transport sctp tun binfmt_misc ext4 jbd2 mbcache loop tcp_diag udp_diag inet_diag rpcrdma ib_isert iscsi_target_mod ib_iser rdma_cm iw_cm libiscsi scsi_transport_iscsi ib_srpt target_core_mod ib_srp scsi_transport_srp ib_ipoib ib_ucm ib_uverbs ib_umad ib_cm ib_core nls_koi8_u nls_cp932 ts_kmp nf_conntrack_ipv4 nf_defrag_ipv4 nf_conntrack vfat fat ghash_ce sha2_ce sha1_ce cavium_rng_vf i2c_thunderx sg thunderx_edac i2c_smbus edac_core cavium_rng nfsd auth_rpcgss nfs_acl lockd grace sunrpc xfs libcrc32c nicvf nicpf ast i2c_algo_bit drm_kms_helper syscopyarea sysfillrect s
ysimgblt fb_sys_fops
> > > > [ 6708.736446] ttm drm i2c_core thunder_bgx thunder_xcv mdio_thunder mdio_cavium dm_mirror dm_region_hash dm_log dm_mod [last unloaded: stap_3c300909c5b3f46dcacd49aab3334af_87021]
> > > > [ 6708.752275] CPU: 84 PID: 0 Comm: swapper/84 Tainted: G W OE 4.11.0-4.el7.aarch64 #1
> > > > [ 6708.760787] Hardware name: www.cavium.com CRB-2S/CRB-2S, BIOS 0.3 Mar 13 2017
> > > > [ 6708.767910] task: ffff810006842e80 task.stack: ffff81000689c000
> > > > [ 6708.773822] PC is at 0x0
> > > > [ 6708.776739] LR is at svc_data_ready+0x38/0x88 [sunrpc]
> > > > [ 6708.781866] pc : [<0000000000000000>] lr : [<ffff0000029d7378>] pstate: 60000145
> > > > [ 6708.789248] sp : ffff810ffbad3900
> > > > [ 6708.792551] x29: ffff810ffbad3900 x28: ffff000008c73d58
> > > > [ 6708.797853] x27: 0000000000000000 x26: ffff81000bbe1e00
> > > > [ 6708.803156] x25: 0000000000000020 x24: ffff800f7410bf28
> > > > [ 6708.808458] x23: ffff000008c63000 x22: ffff000008c63000
> > > > [ 6708.813760] x21: ffff800f7410bf28 x20: ffff81000bbe1e00
> > > > [ 6708.819063] x19: ffff810012412400 x18: 00000000d82a9df2
> > > > [ 6708.824365] x17: 0000000000000000 x16: 0000000000000000
> > > > [ 6708.829667] x15: 0000000000000000 x14: 0000000000000001
> > > > [ 6708.834969] x13: 0000000000000000 x12: 722e736f622e676e
> > > > [ 6708.840271] x11: 00000000f814dd99 x10: 0000000000000000
> > > > [ 6708.845573] x9 : 7374687225000000 x8 : 0000000000000000
> > > > [ 6708.850875] x7 : 0000000000000000 x6 : 0000000000000000
> > > > [ 6708.856177] x5 : 0000000000000028 x4 : 0000000000000000
> > > > [ 6708.861479] x3 : 0000000000000000 x2 : 00000000e5000000
> > > > [ 6708.866781] x1 : 0000000000000000 x0 : ffff81000bbe1e00
> > > > [ 6708.872084]
> > > > [ 6708.873565] Process swapper/84 (pid: 0, stack limit = 0xffff81000689c000)
> > > > [ 6708.880341] Stack: (0xffff810ffbad3900 to 0xffff8100068a0000)
> > > > [ 6708.886075] Call trace:
> > > > [ 6708.888513] Exception stack(0xffff810ffbad3710 to 0xffff810ffbad3840)
> > > > [ 6708.894942] 3700: ffff810012412400 0001000000000000
> > > > [ 6708.902759] 3720: ffff810ffbad3900 0000000000000000 0000000060000145 ffff800f79300000
> > > > [ 6708.910577] 3740: ffff000009274d00 00000000000003ea 0000000000000015 ffff000008c63000
> > > > [ 6708.918395] 3760: ffff810ffbad3830 ffff800f79300000 000000000000004d 0000000000000000
> > > > [ 6708.926212] 3780: ffff810ffbad3890 ffff0000080f88dc ffff800f79300000 000000000000004d
> > > > [ 6708.934030] 37a0: ffff800f7930093c ffff000008c63000 0000000000000000 0000000000000140
> > > > [ 6708.941848] 37c0: ffff000008c2c000 0000000000040b00 ffff81000bbe1e00 0000000000000000
> > > > [ 6708.949665] 37e0: 00000000e5000000 0000000000000000 0000000000000000 0000000000000028
> > > > [ 6708.957483] 3800: 0000000000000000 0000000000000000 0000000000000000 7374687225000000
> > > > [ 6708.965300] 3820: 0000000000000000 00000000f814dd99 722e736f622e676e 0000000000000000
> > > > [ 6708.973117] [< (null)>] (null)
> > > > [ 6708.977824] [<ffff0000086f9fa4>] tcp_data_queue+0x754/0xc5c
> > > > [ 6708.983386] [<ffff0000086fa64c>] tcp_rcv_established+0x1a0/0x67c
> > > > [ 6708.989384] [<ffff000008704120>] tcp_v4_do_rcv+0x15c/0x22c
> > > > [ 6708.994858] [<ffff000008707418>] tcp_v4_rcv+0xaf0/0xb58
> > > > [ 6709.000077] [<ffff0000086df784>] ip_local_deliver_finish+0x10c/0x254
> > > > [ 6709.006419] [<ffff0000086dfea4>] ip_local_deliver+0xf0/0xfc
> > > > [ 6709.011980] [<ffff0000086dfad4>] ip_rcv_finish+0x208/0x3a4
> > > > [ 6709.017454] [<ffff0000086e018c>] ip_rcv+0x2dc/0x3c8
> > > > [ 6709.022328] [<ffff000008692fc8>] __netif_receive_skb_core+0x2f8/0xa0c
> > > > [ 6709.028758] [<ffff000008696068>] __netif_receive_skb+0x38/0x84
> > > > [ 6709.034580] [<ffff00000869611c>] netif_receive_skb_internal+0x68/0xdc
> > > > [ 6709.041010] [<ffff000008696bc0>] napi_gro_receive+0xcc/0x1a8
> > > > [ 6709.046690] [<ffff0000014b0fc4>] nicvf_cq_intr_handler+0x59c/0x730 [nicvf]
> > > > [ 6709.053559] [<ffff0000014b1380>] nicvf_poll+0x38/0xb8 [nicvf]
> > > > [ 6709.059295] [<ffff000008697a6c>] net_rx_action+0x2f8/0x464
> > > > [ 6709.064771] [<ffff000008081824>] __do_softirq+0x11c/0x308
> > > > [ 6709.070164] [<ffff0000080d14e4>] irq_exit+0x12c/0x174
> > > > [ 6709.075206] [<ffff00000813101c>] __handle_domain_irq+0x78/0xc4
> > > > [ 6709.081027] [<ffff000008081608>] gic_handle_irq+0x94/0x190
> > > > [ 6709.086501] Exception stack(0xffff81000689fdf0 to 0xffff81000689ff20)
> > > > [ 6709.092929] fde0: 0000810ff2ec0000 ffff000008c10000
> > > > [ 6709.100747] fe00: ffff000008c70ef4 0000000000000001 0000000000000000 ffff810ffbad9b18
> > > > [ 6709.108565] fe20: ffff810ffbad9c70 ffff8100169d3800 ffff810006843ab0 ffff81000689fe80
> > > > [ 6709.116382] fe40: 0000000000000bd0 0000ffffdf979cd0 183f5913da192500 0000ffff8a254ce4
> > > > [ 6709.124200] fe60: 0000ffff8a254b78 0000aaab10339808 0000000000000000 0000ffff8a0c2a50
> > > > [ 6709.132018] fe80: 0000ffffdf979b10 ffff000008d6d450 ffff000008c10000 ffff000008d6d000
> > > > [ 6709.139836] fea0: 0000000000000054 ffff000008cd3dbc 0000000000000000 0000000000000000
> > > > [ 6709.147653] fec0: 0000000000000000 0000000000000000 0000000000000000 ffff81000689ff20
> > > > [ 6709.155471] fee0: ffff000008085240 ffff81000689ff20 ffff000008085244 0000000060000145
> > > > [ 6709.163289] ff00: ffff81000689ff10 ffff00000813f1e4 ffffffffffffffff ffff00000813f238
> > > > [ 6709.171107] [<ffff000008082eb4>] el1_irq+0xb4/0x140
> > > > [ 6709.175976] [<ffff000008085244>] arch_cpu_idle+0x44/0x11c
> > > > [ 6709.181368] [<ffff0000087bf3b8>] default_idle_call+0x20/0x30
> > > > [ 6709.187020] [<ffff000008116d50>] do_idle+0x158/0x1e4
> > > > [ 6709.191973] [<ffff000008116ff4>] cpu_startup_entry+0x2c/0x30
> > > > [ 6709.197624] [<ffff00000808e7cc>] secondary_start_kernel+0x13c/0x160
> > > > [ 6709.203878] [<0000000001bc71c4>] 0x1bc71c4
> > > > [ 6709.207967] Code: bad PC value
> > > > [ 6709.211061] SMP: stopping secondary CPUs
> > > > [ 6709.218830] Starting crashdump kernel...
> > > > [ 6709.222749] Bye!
> > > > ---<-snip>---
> > > >
> > > > Signed-off-by: Vadim Lomovtsev <vlomovts-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > > > ---
> > > > net/sunrpc/svcsock.c | 24 ++++++++++++++++++------
> > > > 1 file changed, 18 insertions(+), 6 deletions(-)
> > > >
> > > > diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
> > > > index 2b720fa..b6496f3 100644
> > > > --- a/net/sunrpc/svcsock.c
> > > > +++ b/net/sunrpc/svcsock.c
> > > > @@ -421,7 +421,9 @@ static void svc_data_ready(struct sock *sk)
> > > > dprintk("svc: socket %p(inet %p), busy=%d\n",
> > > > svsk, sk,
> > > > test_bit(XPT_BUSY, &svsk->sk_xprt.xpt_flags));
> > > > - svsk->sk_odata(sk);
> > > > + rmb();
> > > > + if (svsk->sk_odata)
> > > > + svsk->sk_odata(sk);
> > >
> > > I'm not convinced these checks will really do much, though you might
> > > need the rmb() calls there...see below...
> > >
> > > > if (!test_and_set_bit(XPT_DATA, &svsk->sk_xprt.xpt_flags))
> > > > svc_xprt_enqueue(&svsk->sk_xprt);
> > > > }
> > > > @@ -437,7 +439,9 @@ static void svc_write_space(struct sock *sk)
> > > > if (svsk) {
> > > > dprintk("svc: socket %p(inet %p), write_space busy=%d\n",
> > > > svsk, sk, test_bit(XPT_BUSY, &svsk->sk_xprt.xpt_flags));
> > > > - svsk->sk_owspace(sk);
> > > > + rmb();
> > > > + if (svsk->sk_owspace)
> > > > + svsk->sk_owspace(sk);
> > > > svc_xprt_enqueue(&svsk->sk_xprt);
> > > > }
> > > > }
> > > > @@ -760,8 +764,12 @@ static void svc_tcp_listen_data_ready(struct sock *sk)
> > > > dprintk("svc: socket %p TCP (listen) state change %d\n",
> > > > sk, sk->sk_state);
> > > >
> > > > - if (svsk)
> > > > - svsk->sk_odata(sk);
> > > > + if (svsk) {
> > > > + rmb();
> > > > + if (svsk->sk_odata)
> > > > + svsk->sk_odata(sk);
> > > > + }
> > > > +
> > > > /*
> > > > * This callback may called twice when a new connection
> > > > * is established as a child socket inherits everything
> > > > @@ -794,7 +802,10 @@ static void svc_tcp_state_change(struct sock *sk)
> > > > if (!svsk)
> > > > printk("svc: socket %p: no user data\n", sk);
> > > > else {
> > > > - svsk->sk_ostate(sk);
> > > > + rmb();
> > > > + if (svsk->sk_ostate)
> > > > + svsk->sk_ostate(sk);
> > > > +
> > > > if (sk->sk_state != TCP_ESTABLISHED) {
> > > > set_bit(XPT_CLOSE, &svsk->sk_xprt.xpt_flags);
> > > > svc_xprt_enqueue(&svsk->sk_xprt);
> > > > @@ -1381,12 +1392,13 @@ static struct svc_sock *svc_setup_socket(struct svc_serv *serv,
> > > > return ERR_PTR(err);
> > > > }
> > > >
> > > > - inet->sk_user_data = svsk;
> > > > svsk->sk_sock = sock;
> > > > svsk->sk_sk = inet;
> > > > svsk->sk_ostate = inet->sk_state_change;
> > > > svsk->sk_odata = inet->sk_data_ready;
> > > > svsk->sk_owspace = inet->sk_write_space;
> > > > + wmb();
> > > > + inet->sk_user_data = svsk;
> > > >
> > > > /* Initialize the socket */
> > > > if (sock->type == SOCK_DGRAM)
> > >
> > > Since we always check whether svsk is NULL before calling the functions,
> > > then the above hunk and maybe the rmb() calls might be all that's needed
> > > here. I don't see how we'd ever get a sk_odata value (for instance)
> > > that's NULL with the above change.
> >
> > Thanks for feed-back. I'll update patch then, run test cycle once again
> > (hope couple days would be enough) and re-send v2 to review.
> >
>
> Great. It might also be nice to sprinkle in some comments that document
> how the rmb/wmb calls are paired together too.
>
Got it. Will do.
> --
> Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
WBR,
Vadim
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH net-next 3/3] net: hns3: Fixes the static check warning due to missing unsupp L3 proto check
From: Salil Mehta @ 2017-08-18 11:31 UTC (permalink / raw)
To: davem
Cc: salil.mehta, yisen.zhuang, lipeng321, dan.carpenter,
mehta.salil.lnk, netdev, linux-kernel, linux-rdma, linuxarm
In-Reply-To: <20170818113139.153200-1-salil.mehta@huawei.com>
This patch fixes the static check warning due to missing handling leg of
unsupported L3 protocol type in the hns3_get_l4_protocol() function.
Fixes: 76ad4f0ee747 ("net: hns3: Add support of HNS3 Ethernet Driver for
hip08 SoC")
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.c
index b12730a..e731f87 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.c
@@ -436,8 +436,8 @@ static int hns3_set_tso(struct sk_buff *skb, u32 *paylen,
return 0;
}
-static void hns3_get_l4_protocol(struct sk_buff *skb, u8 *ol4_proto,
- u8 *il4_proto)
+static int hns3_get_l4_protocol(struct sk_buff *skb, u8 *ol4_proto,
+ u8 *il4_proto)
{
union {
struct iphdr *v4;
@@ -461,6 +461,8 @@ static void hns3_get_l4_protocol(struct sk_buff *skb, u8 *ol4_proto,
&l4_proto_tmp, &frag_off);
} else if (skb->protocol == htons(ETH_P_IP)) {
l4_proto_tmp = l3.v4->protocol;
+ } else {
+ return -EINVAL;
}
*ol4_proto = l4_proto_tmp;
@@ -468,7 +470,7 @@ static void hns3_get_l4_protocol(struct sk_buff *skb, u8 *ol4_proto,
/* tunnel packet */
if (!skb->encapsulation) {
*il4_proto = 0;
- return;
+ return 0;
}
/* find inner header point */
@@ -486,6 +488,8 @@ static void hns3_get_l4_protocol(struct sk_buff *skb, u8 *ol4_proto,
}
*il4_proto = l4_proto_tmp;
+
+ return 0;
}
static void hns3_set_l2l3l4_len(struct sk_buff *skb, u8 ol4_proto,
@@ -757,7 +761,9 @@ static int hns3_fill_desc(struct hns3_enet_ring *ring, void *priv,
protocol = vlan_get_protocol(skb);
skb->protocol = protocol;
}
- hns3_get_l4_protocol(skb, &ol4_proto, &il4_proto);
+ ret = hns3_get_l4_protocol(skb, &ol4_proto, &il4_proto);
+ if (ret)
+ return ret;
hns3_set_l2l3l4_len(skb, ol4_proto, il4_proto,
&type_cs_vlan_tso,
&ol_type_vlan_len_msec);
--
2.7.4
^ permalink raw reply related
* [PATCH net-next 2/3] net: hns3: Fixes the static checker error warning in hns3_get_link_ksettings()
From: Salil Mehta @ 2017-08-18 11:31 UTC (permalink / raw)
To: davem
Cc: salil.mehta, yisen.zhuang, lipeng321, dan.carpenter,
mehta.salil.lnk, netdev, linux-kernel, linux-rdma, linuxarm
In-Reply-To: <20170818113139.153200-1-salil.mehta@huawei.com>
This patch fixes the static check error warning in hns3_get_link_ksettings()
function by re-arranging the code.
Fixes: 496d03e960ae ("net: hns3: Add Ethtool support to HNS3 Driver")
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
.../ethernet/hisilicon/hns3/hns3pf/hns3_ethtool.c | 85 ++++++++++++----------
1 file changed, 48 insertions(+), 37 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_ethtool.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_ethtool.c
index 0ad65e4..ffc837b 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_ethtool.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_ethtool.c
@@ -313,7 +313,7 @@ static int hns3_get_link_ksettings(struct net_device *netdev,
if (!h->ae_algo || !h->ae_algo->ops)
return -EOPNOTSUPP;
- /* 1.auto_neg&speed&duplex from cmd */
+ /* 1.auto_neg & speed & duplex from cmd */
if (h->ae_algo->ops->get_ksettings_an_result) {
h->ae_algo->ops->get_ksettings_an_result(h, &auto_neg,
&speed, &duplex);
@@ -329,50 +329,61 @@ static int hns3_get_link_ksettings(struct net_device *netdev,
}
/* 2.media_type get from bios parameter block */
- if (h->ae_algo->ops->get_media_type)
+ if (h->ae_algo->ops->get_media_type) {
h->ae_algo->ops->get_media_type(h, &media_type);
- switch (media_type) {
- case HNAE3_MEDIA_TYPE_FIBER:
- cmd->base.port = PORT_FIBRE;
- supported_caps = HNS3_LM_FIBRE_BIT | HNS3_LM_AUTONEG_BIT |
- HNS3_LM_PAUSE_BIT | HNS3_LM_1000BASET_FULL_BIT;
+ switch (media_type) {
+ case HNAE3_MEDIA_TYPE_FIBER:
+ cmd->base.port = PORT_FIBRE;
+ supported_caps = HNS3_LM_FIBRE_BIT |
+ HNS3_LM_AUTONEG_BIT |
+ HNS3_LM_PAUSE_BIT |
+ HNS3_LM_1000BASET_FULL_BIT;
+
+ advertised_caps = supported_caps;
+ break;
+ case HNAE3_MEDIA_TYPE_COPPER:
+ cmd->base.port = PORT_TP;
+ supported_caps = HNS3_LM_TP_BIT |
+ HNS3_LM_AUTONEG_BIT |
+ HNS3_LM_PAUSE_BIT |
+ HNS3_LM_1000BASET_FULL_BIT |
+ HNS3_LM_100BASET_FULL_BIT |
+ HNS3_LM_100BASET_HALF_BIT |
+ HNS3_LM_10BASET_FULL_BIT |
+ HNS3_LM_10BASET_HALF_BIT;
+ advertised_caps = supported_caps;
+ break;
+ case HNAE3_MEDIA_TYPE_BACKPLANE:
+ cmd->base.port = PORT_NONE;
+ supported_caps = HNS3_LM_BACKPLANE_BIT |
+ HNS3_LM_PAUSE_BIT |
+ HNS3_LM_AUTONEG_BIT |
+ HNS3_LM_1000BASET_FULL_BIT |
+ HNS3_LM_100BASET_FULL_BIT |
+ HNS3_LM_100BASET_HALF_BIT |
+ HNS3_LM_10BASET_FULL_BIT |
+ HNS3_LM_10BASET_HALF_BIT;
+
+ advertised_caps = supported_caps;
+ break;
+ case HNAE3_MEDIA_TYPE_UNKNOWN:
+ default:
+ cmd->base.port = PORT_OTHER;
+ supported_caps = 0;
+ advertised_caps = 0;
+ break;
+ }
- advertised_caps = supported_caps;
- break;
- case HNAE3_MEDIA_TYPE_COPPER:
- cmd->base.port = PORT_TP;
- supported_caps = HNS3_LM_TP_BIT | HNS3_LM_AUTONEG_BIT |
- HNS3_LM_PAUSE_BIT | HNS3_LM_1000BASET_FULL_BIT |
- HNS3_LM_100BASET_FULL_BIT | HNS3_LM_100BASET_HALF_BIT |
- HNS3_LM_10BASET_FULL_BIT | HNS3_LM_10BASET_HALF_BIT;
- advertised_caps = supported_caps;
- break;
- case HNAE3_MEDIA_TYPE_BACKPLANE:
- cmd->base.port = PORT_NONE;
- supported_caps = HNS3_LM_BACKPLANE_BIT | HNS3_LM_PAUSE_BIT |
- HNS3_LM_AUTONEG_BIT | HNS3_LM_1000BASET_FULL_BIT |
- HNS3_LM_100BASET_FULL_BIT | HNS3_LM_100BASET_HALF_BIT |
- HNS3_LM_10BASET_FULL_BIT | HNS3_LM_10BASET_HALF_BIT;
-
- advertised_caps = supported_caps;
- break;
- case HNAE3_MEDIA_TYPE_UNKNOWN:
- default:
- cmd->base.port = PORT_OTHER;
- supported_caps = 0;
- advertised_caps = 0;
- break;
+ /* now, map driver link modes to ethtool link modes */
+ hns3_driv_to_eth_caps(supported_caps, cmd, false);
+ hns3_driv_to_eth_caps(advertised_caps, cmd, true);
}
- /* now, map driver link modes to ethtool link modes */
- hns3_driv_to_eth_caps(supported_caps, cmd, false);
- hns3_driv_to_eth_caps(advertised_caps, cmd, true);
-
/* 3.mdix_ctrl&mdix get from phy reg */
if (h->ae_algo->ops->get_mdix_mode)
h->ae_algo->ops->get_mdix_mode(h, &cmd->base.eth_tp_mdix_ctrl,
- &cmd->base.eth_tp_mdix);
+ &cmd->base.eth_tp_mdix);
/* 4.mdio_support */
cmd->base.mdio_support = ETH_MDIO_SUPPORTS_C22;
--
2.7.4
^ permalink raw reply related
* [PATCH net-next 1/3] net: hns3: Fixes the missing u64_stats_fetch_begin_irq in 64-bit stats fetch
From: Salil Mehta @ 2017-08-18 11:31 UTC (permalink / raw)
To: davem
Cc: salil.mehta, yisen.zhuang, lipeng321, dan.carpenter,
mehta.salil.lnk, netdev, linux-kernel, linux-rdma, linuxarm
In-Reply-To: <20170818113139.153200-1-salil.mehta@huawei.com>
This patch fixes the missing u64_stats_fetch_begin_irq() while trying to
atomically do 64-bit RX/TX fetch. We did not get any error during test
as our SoC is 64-bit so all of these seq/lock operations results in NOOP.
As such, this seq lock supports has been added for the sake of completion
if this code ever runs on 32-bit platform and we are trying to do 64-bit
stats fetch.
Fixes: 76ad4f0ee747 ("net: hns3: Add support of HNS3 Ethernet Driver for
hip08 SoC")
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.c
index 9589b7e..b12730a 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.c
@@ -1054,6 +1054,7 @@ hns3_nic_get_stats64(struct net_device *netdev, struct rtnl_link_stats64 *stats)
/* fetch the tx stats */
ring = priv->ring_data[idx].ring;
do {
+ start = u64_stats_fetch_begin_irq(&ring->syncp);
tx_bytes += ring->stats.tx_bytes;
tx_pkts += ring->stats.tx_pkts;
} while (u64_stats_fetch_retry_irq(&ring->syncp, start));
@@ -1061,6 +1062,7 @@ hns3_nic_get_stats64(struct net_device *netdev, struct rtnl_link_stats64 *stats)
/* fetch the rx stats */
ring = priv->ring_data[idx + queue_num].ring;
do {
+ start = u64_stats_fetch_begin_irq(&ring->syncp);
rx_bytes += ring->stats.rx_bytes;
rx_pkts += ring->stats.rx_pkts;
} while (u64_stats_fetch_retry_irq(&ring->syncp, start));
--
2.7.4
^ permalink raw reply related
* [PATCH net-next 0/3] Misc. Bug fixes for HNS3 Ethernet Driver
From: Salil Mehta @ 2017-08-18 11:31 UTC (permalink / raw)
To: davem-fT/PcQaiUtIeIZ0/mPfg9Q
Cc: salil.mehta-hv44wF8Li93QT0dZR+AlfA,
yisen.zhuang-hv44wF8Li93QT0dZR+AlfA,
lipeng321-hv44wF8Li93QT0dZR+AlfA,
dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA,
mehta.salil.lnk-Re5JQEeQqe8AvxtiuMwx3w,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linuxarm-hv44wF8Li93QT0dZR+AlfA
This patch-set fixes various bugs reported by community.
Salil Mehta (3):
net: hns3: Fixes the missing u64_stats_fetch_begin_irq in 64-bit stats
fetch
net: hns3: Fixes the static checker error warning in
hns3_get_link_ksettings()
net: hns3: Fixes the static check warning due to missing unsupp L3
proto check
.../net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.c | 16 +++-
.../ethernet/hisilicon/hns3/hns3pf/hns3_ethtool.c | 85 ++++++++++++----------
2 files changed, 60 insertions(+), 41 deletions(-)
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [patch net-next] net/sched: Fix the logic error to decide the ingress qdisc
From: Chris Mi @ 2017-08-18 11:24 UTC (permalink / raw)
To: netdev; +Cc: davem, jiri
The offending commit used a newly added helper function.
But the logic is wrong. Without this fix, the affected NICs
can't do HW offload. Error -EOPNOTSUPP will be returned directly.
Fixes: a2e8da9378cc ("net/sched: use newly added classid identity helpers")
Signed-off-by: Chris Mi <chrism@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 2 +-
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 2 +-
drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 2 +-
drivers/net/ethernet/mellanox/mlx5/core/en_rep.c | 2 +-
drivers/net/ethernet/netronome/nfp/bpf/main.c | 2 +-
drivers/net/ethernet/netronome/nfp/flower/offload.c | 2 +-
6 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
index 77538cd..e55a929 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
@@ -2892,7 +2892,7 @@ static int cxgb_set_tx_maxrate(struct net_device *dev, int index, u32 rate)
static int cxgb_setup_tc_cls_u32(struct net_device *dev,
struct tc_cls_u32_offload *cls_u32)
{
- if (is_classid_clsact_ingress(cls_u32->common.classid) ||
+ if (!is_classid_clsact_ingress(cls_u32->common.classid) ||
cls_u32->common.chain_index)
return -EOPNOTSUPP;
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index f9fd8d8..56d7ef0 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -9230,7 +9230,7 @@ static int ixgbe_setup_tc_cls_u32(struct net_device *dev,
{
struct ixgbe_adapter *adapter = netdev_priv(dev);
- if (is_classid_clsact_ingress(cls_u32->common.classid) ||
+ if (!is_classid_clsact_ingress(cls_u32->common.classid) ||
cls_u32->common.chain_index)
return -EOPNOTSUPP;
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index 8633ca5..2fc3832 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -3031,7 +3031,7 @@ static int mlx5e_setup_tc_cls_flower(struct net_device *dev,
{
struct mlx5e_priv *priv = netdev_priv(dev);
- if (is_classid_clsact_ingress(cls_flower->common.classid) ||
+ if (!is_classid_clsact_ingress(cls_flower->common.classid) ||
cls_flower->common.chain_index)
return -EOPNOTSUPP;
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
index f34c00f..7a9f53f 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
@@ -657,7 +657,7 @@ static int mlx5e_rep_get_phys_port_name(struct net_device *dev,
{
struct mlx5e_priv *priv = netdev_priv(dev);
- if (is_classid_clsact_ingress(cls_flower->common.classid) ||
+ if (!is_classid_clsact_ingress(cls_flower->common.classid) ||
cls_flower->common.chain_index)
return -EOPNOTSUPP;
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/main.c b/drivers/net/ethernet/netronome/nfp/bpf/main.c
index 0e68649..f4de3a7 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/main.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/main.c
@@ -127,7 +127,7 @@ static int nfp_bpf_setup_tc(struct nfp_app *app, struct net_device *netdev,
struct nfp_net *nn = netdev_priv(netdev);
if (type != TC_SETUP_CLSBPF || !nfp_net_ebpf_capable(nn) ||
- is_classid_clsact_ingress(cls_bpf->common.classid) ||
+ !is_classid_clsact_ingress(cls_bpf->common.classid) ||
cls_bpf->common.protocol != htons(ETH_P_ALL) ||
cls_bpf->common.chain_index)
return -EOPNOTSUPP;
diff --git a/drivers/net/ethernet/netronome/nfp/flower/offload.c b/drivers/net/ethernet/netronome/nfp/flower/offload.c
index 3ad5aaa..d868a57 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/offload.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/offload.c
@@ -390,7 +390,7 @@ int nfp_flower_setup_tc(struct nfp_app *app, struct net_device *netdev,
struct tc_cls_flower_offload *cls_flower = type_data;
if (type != TC_SETUP_CLSFLOWER ||
- is_classid_clsact_ingress(cls_flower->common.classid) ||
+ !is_classid_clsact_ingress(cls_flower->common.classid) ||
!eth_proto_is_802_3(cls_flower->common.protocol) ||
cls_flower->common.chain_index)
return -EOPNOTSUPP;
--
1.8.3.1
^ permalink raw reply related
* Re: [iproute PATCH v2 2/2] ifcfg: Quote left-hand side of [ ] expression
From: Phil Sutter @ 2017-08-18 11:24 UTC (permalink / raw)
To: David Laight; +Cc: Stephen Hemminger, netdev@vger.kernel.org
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DD005A809@AcuExch.aculab.com>
On Fri, Aug 18, 2017 at 09:32:52AM +0000, David Laight wrote:
> From: Phil Sutter
> > Sent: 17 August 2017 18:10
> > This prevents word-splitting and therefore leads to more accurate error
> > message in case 'grep -c' prints something other than a number.
> >
> > Signed-off-by: Phil Sutter <phil@nwl.cc>
> > ---
> > ip/ifcfg | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/ip/ifcfg b/ip/ifcfg
> > index 083d9df36742f..30a2dc49816cd 100644
> > --- a/ip/ifcfg
> > +++ b/ip/ifcfg
> > @@ -131,7 +131,7 @@ noarp=$?
> >
> > ip route add unreachable 224.0.0.0/24 >& /dev/null
> > ip route add unreachable 255.255.255.255 >& /dev/null
> > -if [ `ip link ls $dev | grep -c MULTICAST` -ge 1 ]; then
> > +if [ "`ip link ls $dev | grep -c MULTICAST`" -ge 1 ]; then
>
> You could drag all these scripts into the 1990's by using $(...)
> instead of `...`.
That's a different kettle of fish IMO, using $() doesn't change the
situation this addresses:
| $ [ $(echo foo bar) -eq 0 ] && echo ok
| bash: [: too many arguments
| $ [ "$(echo foo bar)" -eq 0 ] && echo ok
| bash: [: foo bar: integer expression expected
Cheers, Phil
^ permalink raw reply
* Re: [iproute PATCH v2 2/3] iproute_lwtunnel: Argument to strerror must be positive
From: Phil Sutter @ 2017-08-18 11:21 UTC (permalink / raw)
To: David Laight; +Cc: Stephen Hemminger, netdev@vger.kernel.org
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DD005A7C8@AcuExch.aculab.com>
On Fri, Aug 18, 2017 at 09:21:34AM +0000, David Laight wrote:
> From: Phil Sutter
> > Sent: 17 August 2017 18:10
> > Signed-off-by: Phil Sutter <phil@nwl.cc>
> > ---
> > ip/iproute_lwtunnel.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/ip/iproute_lwtunnel.c b/ip/iproute_lwtunnel.c
> > index 398ab5e077ed8..1a3dc4d4c0ed9 100644
> > --- a/ip/iproute_lwtunnel.c
> > +++ b/ip/iproute_lwtunnel.c
> > @@ -643,7 +643,7 @@ static int lwt_parse_bpf(struct rtattr *rta, size_t len,
> > err = bpf_parse_common(bpf_type, &cfg, &bpf_cb_ops, &x);
> > if (err < 0) {
> > fprintf(stderr, "Failed to parse eBPF program: %s\n",
> > - strerror(err));
> > + strerror(-err));
>
> If we are in userspace I'd expect errno values to be +ve.
> Returning a -ve errno is very non-standard.
This is because bpf_parse() returns the number of instructions parsed or
a negative return code. We could change it to return instructions * -1
or a positive return code, but that's even more insane than calling
strerror(-err), isn't it?
Cheers, Phil
^ permalink raw reply
* Re: [PATCH] net: sunrpc: svcsock: fix NULL-pointer exception
From: Jeff Layton @ 2017-08-18 11:16 UTC (permalink / raw)
To: Vadim Lomovtsev
Cc: trond.myklebust, anna.schumaker, bfields, davem, linux-nfs,
netdev, linux-kernel, pabeni
In-Reply-To: <20170818110817.GA24643@dhcp187-32.khw.lab.eng.bos.redhat.com>
On Fri, 2017-08-18 at 07:08 -0400, Vadim Lomovtsev wrote:
> Hi Jeff,
>
> On Fri, Aug 18, 2017 at 06:27:32AM -0400, Jeff Layton wrote:
> > On Fri, 2017-08-18 at 06:00 -0400, Vadim Lomovtsev wrote:
> > > While running nfs/connectathon tests kernel NULL-pointer exception
> > > has been observed due to races in svcsock.c.
> > >
> > > Race is appear when kernel accepts connection by kernel_accept
> > > (which creates new socket) and start queuing ingress packets
> > > to new socket. This happanes in ksoftirq context which concurrently
> > > on a differnt core while new socket setup is not done yet.
> > >
> > > The fix is to re-order socket user data init sequence, add NULL-ptr
> > > check before callback call along with barriers to prevent kernel crash.
> > >
> > > Test results: nfs/connectathon reports '0' failed tests for about 200+ iterations.
> > >
> > > Crash log:
> > > ---<-snip->---
> > > [ 6708.638984] Unable to handle kernel NULL pointer dereference at virtual address 00000000
> > > [ 6708.647093] pgd = ffff0000094e0000
> > > [ 6708.650497] [00000000] *pgd=0000010ffff90003, *pud=0000010ffff90003, *pmd=0000010ffff80003, *pte=0000000000000000
> > > [ 6708.660761] Internal error: Oops: 86000005 [#1] SMP
> > > [ 6708.665630] Modules linked in: nfsv3 nfnetlink_queue nfnetlink_log nfnetlink rpcsec_gss_krb5 nfsv4 dns_resolver nfs fscache overlay xt_CONNSECMARK xt_SECMARK xt_conntrack iptable_security ip_tables ah4 xfrm4_mode_transport sctp tun binfmt_misc ext4 jbd2 mbcache loop tcp_diag udp_diag inet_diag rpcrdma ib_isert iscsi_target_mod ib_iser rdma_cm iw_cm libiscsi scsi_transport_iscsi ib_srpt target_core_mod ib_srp scsi_transport_srp ib_ipoib ib_ucm ib_uverbs ib_umad ib_cm ib_core nls_koi8_u nls_cp932 ts_kmp nf_conntrack_ipv4 nf_defrag_ipv4 nf_conntrack vfat fat ghash_ce sha2_ce sha1_ce cavium_rng_vf i2c_thunderx sg thunderx_edac i2c_smbus edac_core cavium_rng nfsd auth_rpcgss nfs_acl lockd grace sunrpc xfs libcrc32c nicvf nicpf ast i2c_algo_bit drm_kms_helper syscopyarea sysfillrect sys
imgblt fb_sys_fops
> > > [ 6708.736446] ttm drm i2c_core thunder_bgx thunder_xcv mdio_thunder mdio_cavium dm_mirror dm_region_hash dm_log dm_mod [last unloaded: stap_3c300909c5b3f46dcacd49aab3334af_87021]
> > > [ 6708.752275] CPU: 84 PID: 0 Comm: swapper/84 Tainted: G W OE 4.11.0-4.el7.aarch64 #1
> > > [ 6708.760787] Hardware name: www.cavium.com CRB-2S/CRB-2S, BIOS 0.3 Mar 13 2017
> > > [ 6708.767910] task: ffff810006842e80 task.stack: ffff81000689c000
> > > [ 6708.773822] PC is at 0x0
> > > [ 6708.776739] LR is at svc_data_ready+0x38/0x88 [sunrpc]
> > > [ 6708.781866] pc : [<0000000000000000>] lr : [<ffff0000029d7378>] pstate: 60000145
> > > [ 6708.789248] sp : ffff810ffbad3900
> > > [ 6708.792551] x29: ffff810ffbad3900 x28: ffff000008c73d58
> > > [ 6708.797853] x27: 0000000000000000 x26: ffff81000bbe1e00
> > > [ 6708.803156] x25: 0000000000000020 x24: ffff800f7410bf28
> > > [ 6708.808458] x23: ffff000008c63000 x22: ffff000008c63000
> > > [ 6708.813760] x21: ffff800f7410bf28 x20: ffff81000bbe1e00
> > > [ 6708.819063] x19: ffff810012412400 x18: 00000000d82a9df2
> > > [ 6708.824365] x17: 0000000000000000 x16: 0000000000000000
> > > [ 6708.829667] x15: 0000000000000000 x14: 0000000000000001
> > > [ 6708.834969] x13: 0000000000000000 x12: 722e736f622e676e
> > > [ 6708.840271] x11: 00000000f814dd99 x10: 0000000000000000
> > > [ 6708.845573] x9 : 7374687225000000 x8 : 0000000000000000
> > > [ 6708.850875] x7 : 0000000000000000 x6 : 0000000000000000
> > > [ 6708.856177] x5 : 0000000000000028 x4 : 0000000000000000
> > > [ 6708.861479] x3 : 0000000000000000 x2 : 00000000e5000000
> > > [ 6708.866781] x1 : 0000000000000000 x0 : ffff81000bbe1e00
> > > [ 6708.872084]
> > > [ 6708.873565] Process swapper/84 (pid: 0, stack limit = 0xffff81000689c000)
> > > [ 6708.880341] Stack: (0xffff810ffbad3900 to 0xffff8100068a0000)
> > > [ 6708.886075] Call trace:
> > > [ 6708.888513] Exception stack(0xffff810ffbad3710 to 0xffff810ffbad3840)
> > > [ 6708.894942] 3700: ffff810012412400 0001000000000000
> > > [ 6708.902759] 3720: ffff810ffbad3900 0000000000000000 0000000060000145 ffff800f79300000
> > > [ 6708.910577] 3740: ffff000009274d00 00000000000003ea 0000000000000015 ffff000008c63000
> > > [ 6708.918395] 3760: ffff810ffbad3830 ffff800f79300000 000000000000004d 0000000000000000
> > > [ 6708.926212] 3780: ffff810ffbad3890 ffff0000080f88dc ffff800f79300000 000000000000004d
> > > [ 6708.934030] 37a0: ffff800f7930093c ffff000008c63000 0000000000000000 0000000000000140
> > > [ 6708.941848] 37c0: ffff000008c2c000 0000000000040b00 ffff81000bbe1e00 0000000000000000
> > > [ 6708.949665] 37e0: 00000000e5000000 0000000000000000 0000000000000000 0000000000000028
> > > [ 6708.957483] 3800: 0000000000000000 0000000000000000 0000000000000000 7374687225000000
> > > [ 6708.965300] 3820: 0000000000000000 00000000f814dd99 722e736f622e676e 0000000000000000
> > > [ 6708.973117] [< (null)>] (null)
> > > [ 6708.977824] [<ffff0000086f9fa4>] tcp_data_queue+0x754/0xc5c
> > > [ 6708.983386] [<ffff0000086fa64c>] tcp_rcv_established+0x1a0/0x67c
> > > [ 6708.989384] [<ffff000008704120>] tcp_v4_do_rcv+0x15c/0x22c
> > > [ 6708.994858] [<ffff000008707418>] tcp_v4_rcv+0xaf0/0xb58
> > > [ 6709.000077] [<ffff0000086df784>] ip_local_deliver_finish+0x10c/0x254
> > > [ 6709.006419] [<ffff0000086dfea4>] ip_local_deliver+0xf0/0xfc
> > > [ 6709.011980] [<ffff0000086dfad4>] ip_rcv_finish+0x208/0x3a4
> > > [ 6709.017454] [<ffff0000086e018c>] ip_rcv+0x2dc/0x3c8
> > > [ 6709.022328] [<ffff000008692fc8>] __netif_receive_skb_core+0x2f8/0xa0c
> > > [ 6709.028758] [<ffff000008696068>] __netif_receive_skb+0x38/0x84
> > > [ 6709.034580] [<ffff00000869611c>] netif_receive_skb_internal+0x68/0xdc
> > > [ 6709.041010] [<ffff000008696bc0>] napi_gro_receive+0xcc/0x1a8
> > > [ 6709.046690] [<ffff0000014b0fc4>] nicvf_cq_intr_handler+0x59c/0x730 [nicvf]
> > > [ 6709.053559] [<ffff0000014b1380>] nicvf_poll+0x38/0xb8 [nicvf]
> > > [ 6709.059295] [<ffff000008697a6c>] net_rx_action+0x2f8/0x464
> > > [ 6709.064771] [<ffff000008081824>] __do_softirq+0x11c/0x308
> > > [ 6709.070164] [<ffff0000080d14e4>] irq_exit+0x12c/0x174
> > > [ 6709.075206] [<ffff00000813101c>] __handle_domain_irq+0x78/0xc4
> > > [ 6709.081027] [<ffff000008081608>] gic_handle_irq+0x94/0x190
> > > [ 6709.086501] Exception stack(0xffff81000689fdf0 to 0xffff81000689ff20)
> > > [ 6709.092929] fde0: 0000810ff2ec0000 ffff000008c10000
> > > [ 6709.100747] fe00: ffff000008c70ef4 0000000000000001 0000000000000000 ffff810ffbad9b18
> > > [ 6709.108565] fe20: ffff810ffbad9c70 ffff8100169d3800 ffff810006843ab0 ffff81000689fe80
> > > [ 6709.116382] fe40: 0000000000000bd0 0000ffffdf979cd0 183f5913da192500 0000ffff8a254ce4
> > > [ 6709.124200] fe60: 0000ffff8a254b78 0000aaab10339808 0000000000000000 0000ffff8a0c2a50
> > > [ 6709.132018] fe80: 0000ffffdf979b10 ffff000008d6d450 ffff000008c10000 ffff000008d6d000
> > > [ 6709.139836] fea0: 0000000000000054 ffff000008cd3dbc 0000000000000000 0000000000000000
> > > [ 6709.147653] fec0: 0000000000000000 0000000000000000 0000000000000000 ffff81000689ff20
> > > [ 6709.155471] fee0: ffff000008085240 ffff81000689ff20 ffff000008085244 0000000060000145
> > > [ 6709.163289] ff00: ffff81000689ff10 ffff00000813f1e4 ffffffffffffffff ffff00000813f238
> > > [ 6709.171107] [<ffff000008082eb4>] el1_irq+0xb4/0x140
> > > [ 6709.175976] [<ffff000008085244>] arch_cpu_idle+0x44/0x11c
> > > [ 6709.181368] [<ffff0000087bf3b8>] default_idle_call+0x20/0x30
> > > [ 6709.187020] [<ffff000008116d50>] do_idle+0x158/0x1e4
> > > [ 6709.191973] [<ffff000008116ff4>] cpu_startup_entry+0x2c/0x30
> > > [ 6709.197624] [<ffff00000808e7cc>] secondary_start_kernel+0x13c/0x160
> > > [ 6709.203878] [<0000000001bc71c4>] 0x1bc71c4
> > > [ 6709.207967] Code: bad PC value
> > > [ 6709.211061] SMP: stopping secondary CPUs
> > > [ 6709.218830] Starting crashdump kernel...
> > > [ 6709.222749] Bye!
> > > ---<-snip>---
> > >
> > > Signed-off-by: Vadim Lomovtsev <vlomovts@redhat.com>
> > > ---
> > > net/sunrpc/svcsock.c | 24 ++++++++++++++++++------
> > > 1 file changed, 18 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
> > > index 2b720fa..b6496f3 100644
> > > --- a/net/sunrpc/svcsock.c
> > > +++ b/net/sunrpc/svcsock.c
> > > @@ -421,7 +421,9 @@ static void svc_data_ready(struct sock *sk)
> > > dprintk("svc: socket %p(inet %p), busy=%d\n",
> > > svsk, sk,
> > > test_bit(XPT_BUSY, &svsk->sk_xprt.xpt_flags));
> > > - svsk->sk_odata(sk);
> > > + rmb();
> > > + if (svsk->sk_odata)
> > > + svsk->sk_odata(sk);
> >
> > I'm not convinced these checks will really do much, though you might
> > need the rmb() calls there...see below...
> >
> > > if (!test_and_set_bit(XPT_DATA, &svsk->sk_xprt.xpt_flags))
> > > svc_xprt_enqueue(&svsk->sk_xprt);
> > > }
> > > @@ -437,7 +439,9 @@ static void svc_write_space(struct sock *sk)
> > > if (svsk) {
> > > dprintk("svc: socket %p(inet %p), write_space busy=%d\n",
> > > svsk, sk, test_bit(XPT_BUSY, &svsk->sk_xprt.xpt_flags));
> > > - svsk->sk_owspace(sk);
> > > + rmb();
> > > + if (svsk->sk_owspace)
> > > + svsk->sk_owspace(sk);
> > > svc_xprt_enqueue(&svsk->sk_xprt);
> > > }
> > > }
> > > @@ -760,8 +764,12 @@ static void svc_tcp_listen_data_ready(struct sock *sk)
> > > dprintk("svc: socket %p TCP (listen) state change %d\n",
> > > sk, sk->sk_state);
> > >
> > > - if (svsk)
> > > - svsk->sk_odata(sk);
> > > + if (svsk) {
> > > + rmb();
> > > + if (svsk->sk_odata)
> > > + svsk->sk_odata(sk);
> > > + }
> > > +
> > > /*
> > > * This callback may called twice when a new connection
> > > * is established as a child socket inherits everything
> > > @@ -794,7 +802,10 @@ static void svc_tcp_state_change(struct sock *sk)
> > > if (!svsk)
> > > printk("svc: socket %p: no user data\n", sk);
> > > else {
> > > - svsk->sk_ostate(sk);
> > > + rmb();
> > > + if (svsk->sk_ostate)
> > > + svsk->sk_ostate(sk);
> > > +
> > > if (sk->sk_state != TCP_ESTABLISHED) {
> > > set_bit(XPT_CLOSE, &svsk->sk_xprt.xpt_flags);
> > > svc_xprt_enqueue(&svsk->sk_xprt);
> > > @@ -1381,12 +1392,13 @@ static struct svc_sock *svc_setup_socket(struct svc_serv *serv,
> > > return ERR_PTR(err);
> > > }
> > >
> > > - inet->sk_user_data = svsk;
> > > svsk->sk_sock = sock;
> > > svsk->sk_sk = inet;
> > > svsk->sk_ostate = inet->sk_state_change;
> > > svsk->sk_odata = inet->sk_data_ready;
> > > svsk->sk_owspace = inet->sk_write_space;
> > > + wmb();
> > > + inet->sk_user_data = svsk;
> > >
> > > /* Initialize the socket */
> > > if (sock->type == SOCK_DGRAM)
> >
> > Since we always check whether svsk is NULL before calling the functions,
> > then the above hunk and maybe the rmb() calls might be all that's needed
> > here. I don't see how we'd ever get a sk_odata value (for instance)
> > that's NULL with the above change.
>
> Thanks for feed-back. I'll update patch then, run test cycle once again
> (hope couple days would be enough) and re-send v2 to review.
>
Great. It might also be nice to sprinkle in some comments that document
how the rmb/wmb calls are paired together too.
--
Jeff Layton <jlayton@redhat.com>
^ permalink raw reply
* Re: [iproute PATCH v2 4/5] tc/tc_filter: Make sure filter name is not empty
From: Phil Sutter @ 2017-08-18 11:16 UTC (permalink / raw)
To: David Laight; +Cc: Stephen Hemminger, netdev@vger.kernel.org
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DD005A7E9@AcuExch.aculab.com>
On Fri, Aug 18, 2017 at 09:30:35AM +0000, David Laight wrote:
> From: Phil Sutter
> > Sent: 17 August 2017 18:10
> > The later check for 'k[0] != 0' requires a non-empty filter name,
> > otherwise NULL pointer dereference in 'q' might happen.
> >
> > Signed-off-by: Phil Sutter <phil@nwl.cc>
> > ---
> > tc/tc_filter.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/tc/tc_filter.c b/tc/tc_filter.c
> > index b13fb9185d4fd..a799edb35886d 100644
> > --- a/tc/tc_filter.c
> > +++ b/tc/tc_filter.c
> > @@ -412,6 +412,9 @@ static int tc_filter_get(int cmd, unsigned int flags, int argc, char **argv)
> > usage();
> > return 0;
> > } else {
> > + if (!strlen(*argv))
> > + invarg("invalid filter name", *argv);
>
> That is nearly as bad as:
> p[strlen(p)] = 0;
Hey, it's not impossible! I could call tc like so:
| # tc filter get protocol ip prio 1 ""
What's funny about it is that the first call to matches() in
tc_filter_get() will catch the empty last parameter:
| # tc filter get prio 1 protocol ip ""
| Command line is not complete. Try option "help"
| # ./tc/tc filter get prio 1 protocol ip "" ""
| Error: duplicate "priority": "" is the second value.
Cheers, Phil
^ permalink raw reply
* [PATCH] nfp: fix infinite loop on umapping cleanup
From: Colin King @ 2017-08-18 11:11 UTC (permalink / raw)
To: David S . Miller, Simon Horman, Daniel Borkmann, oss-drivers,
netdev
Cc: Jakub Kicinski, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
The while loop that performs the dma page unmapping never decrements
index counter f and hence loops forever. Fix this with a pre-decrement
on f.
Detected by CoverityScan, CID#1357309 ("Infinite loop")
Fixes: 4c3523623dc0 ("net: add driver for Netronome NFP4000/NFP6000 NIC VFs")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/net/ethernet/netronome/nfp/nfp_net_common.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
index 4a990033c4d5..732f1d315fba 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
@@ -908,8 +908,7 @@ static int nfp_net_tx(struct sk_buff *skb, struct net_device *netdev)
return NETDEV_TX_OK;
err_unmap:
- --f;
- while (f >= 0) {
+ while (--f >= 0) {
frag = &skb_shinfo(skb)->frags[f];
dma_unmap_page(dp->dev, tx_ring->txbufs[wr_idx].dma_addr,
skb_frag_size(frag), DMA_TO_DEVICE);
--
2.11.0
^ permalink raw reply related
* Re: [PATCH] net: sunrpc: svcsock: fix NULL-pointer exception
From: Vadim Lomovtsev @ 2017-08-18 11:08 UTC (permalink / raw)
To: Jeff Layton
Cc: trond.myklebust-7I+n7zu2hftEKMMhf/gKZA,
anna.schumaker-HgOvQuBEEgTQT0dZR+AlfA,
bfields-uC3wQj2KruNg9hUCZPvPmw, davem-fT/PcQaiUtIeIZ0/mPfg9Q,
linux-nfs-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
pabeni-H+wXaHxf7aLQT0dZR+AlfA, vlomovts-H+wXaHxf7aLQT0dZR+AlfA
In-Reply-To: <1503052052.4719.15.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Hi Jeff,
On Fri, Aug 18, 2017 at 06:27:32AM -0400, Jeff Layton wrote:
> On Fri, 2017-08-18 at 06:00 -0400, Vadim Lomovtsev wrote:
> > While running nfs/connectathon tests kernel NULL-pointer exception
> > has been observed due to races in svcsock.c.
> >
> > Race is appear when kernel accepts connection by kernel_accept
> > (which creates new socket) and start queuing ingress packets
> > to new socket. This happanes in ksoftirq context which concurrently
> > on a differnt core while new socket setup is not done yet.
> >
> > The fix is to re-order socket user data init sequence, add NULL-ptr
> > check before callback call along with barriers to prevent kernel crash.
> >
> > Test results: nfs/connectathon reports '0' failed tests for about 200+ iterations.
> >
> > Crash log:
> > ---<-snip->---
> > [ 6708.638984] Unable to handle kernel NULL pointer dereference at virtual address 00000000
> > [ 6708.647093] pgd = ffff0000094e0000
> > [ 6708.650497] [00000000] *pgd=0000010ffff90003, *pud=0000010ffff90003, *pmd=0000010ffff80003, *pte=0000000000000000
> > [ 6708.660761] Internal error: Oops: 86000005 [#1] SMP
> > [ 6708.665630] Modules linked in: nfsv3 nfnetlink_queue nfnetlink_log nfnetlink rpcsec_gss_krb5 nfsv4 dns_resolver nfs fscache overlay xt_CONNSECMARK xt_SECMARK xt_conntrack iptable_security ip_tables ah4 xfrm4_mode_transport sctp tun binfmt_misc ext4 jbd2 mbcache loop tcp_diag udp_diag inet_diag rpcrdma ib_isert iscsi_target_mod ib_iser rdma_cm iw_cm libiscsi scsi_transport_iscsi ib_srpt target_core_mod ib_srp scsi_transport_srp ib_ipoib ib_ucm ib_uverbs ib_umad ib_cm ib_core nls_koi8_u nls_cp932 ts_kmp nf_conntrack_ipv4 nf_defrag_ipv4 nf_conntrack vfat fat ghash_ce sha2_ce sha1_ce cavium_rng_vf i2c_thunderx sg thunderx_edac i2c_smbus edac_core cavium_rng nfsd auth_rpcgss nfs_acl lockd grace sunrpc xfs libcrc32c nicvf nicpf ast i2c_algo_bit drm_kms_helper syscopyarea sysfillrect sysim
gblt fb_sys_fops
> > [ 6708.736446] ttm drm i2c_core thunder_bgx thunder_xcv mdio_thunder mdio_cavium dm_mirror dm_region_hash dm_log dm_mod [last unloaded: stap_3c300909c5b3f46dcacd49aab3334af_87021]
> > [ 6708.752275] CPU: 84 PID: 0 Comm: swapper/84 Tainted: G W OE 4.11.0-4.el7.aarch64 #1
> > [ 6708.760787] Hardware name: www.cavium.com CRB-2S/CRB-2S, BIOS 0.3 Mar 13 2017
> > [ 6708.767910] task: ffff810006842e80 task.stack: ffff81000689c000
> > [ 6708.773822] PC is at 0x0
> > [ 6708.776739] LR is at svc_data_ready+0x38/0x88 [sunrpc]
> > [ 6708.781866] pc : [<0000000000000000>] lr : [<ffff0000029d7378>] pstate: 60000145
> > [ 6708.789248] sp : ffff810ffbad3900
> > [ 6708.792551] x29: ffff810ffbad3900 x28: ffff000008c73d58
> > [ 6708.797853] x27: 0000000000000000 x26: ffff81000bbe1e00
> > [ 6708.803156] x25: 0000000000000020 x24: ffff800f7410bf28
> > [ 6708.808458] x23: ffff000008c63000 x22: ffff000008c63000
> > [ 6708.813760] x21: ffff800f7410bf28 x20: ffff81000bbe1e00
> > [ 6708.819063] x19: ffff810012412400 x18: 00000000d82a9df2
> > [ 6708.824365] x17: 0000000000000000 x16: 0000000000000000
> > [ 6708.829667] x15: 0000000000000000 x14: 0000000000000001
> > [ 6708.834969] x13: 0000000000000000 x12: 722e736f622e676e
> > [ 6708.840271] x11: 00000000f814dd99 x10: 0000000000000000
> > [ 6708.845573] x9 : 7374687225000000 x8 : 0000000000000000
> > [ 6708.850875] x7 : 0000000000000000 x6 : 0000000000000000
> > [ 6708.856177] x5 : 0000000000000028 x4 : 0000000000000000
> > [ 6708.861479] x3 : 0000000000000000 x2 : 00000000e5000000
> > [ 6708.866781] x1 : 0000000000000000 x0 : ffff81000bbe1e00
> > [ 6708.872084]
> > [ 6708.873565] Process swapper/84 (pid: 0, stack limit = 0xffff81000689c000)
> > [ 6708.880341] Stack: (0xffff810ffbad3900 to 0xffff8100068a0000)
> > [ 6708.886075] Call trace:
> > [ 6708.888513] Exception stack(0xffff810ffbad3710 to 0xffff810ffbad3840)
> > [ 6708.894942] 3700: ffff810012412400 0001000000000000
> > [ 6708.902759] 3720: ffff810ffbad3900 0000000000000000 0000000060000145 ffff800f79300000
> > [ 6708.910577] 3740: ffff000009274d00 00000000000003ea 0000000000000015 ffff000008c63000
> > [ 6708.918395] 3760: ffff810ffbad3830 ffff800f79300000 000000000000004d 0000000000000000
> > [ 6708.926212] 3780: ffff810ffbad3890 ffff0000080f88dc ffff800f79300000 000000000000004d
> > [ 6708.934030] 37a0: ffff800f7930093c ffff000008c63000 0000000000000000 0000000000000140
> > [ 6708.941848] 37c0: ffff000008c2c000 0000000000040b00 ffff81000bbe1e00 0000000000000000
> > [ 6708.949665] 37e0: 00000000e5000000 0000000000000000 0000000000000000 0000000000000028
> > [ 6708.957483] 3800: 0000000000000000 0000000000000000 0000000000000000 7374687225000000
> > [ 6708.965300] 3820: 0000000000000000 00000000f814dd99 722e736f622e676e 0000000000000000
> > [ 6708.973117] [< (null)>] (null)
> > [ 6708.977824] [<ffff0000086f9fa4>] tcp_data_queue+0x754/0xc5c
> > [ 6708.983386] [<ffff0000086fa64c>] tcp_rcv_established+0x1a0/0x67c
> > [ 6708.989384] [<ffff000008704120>] tcp_v4_do_rcv+0x15c/0x22c
> > [ 6708.994858] [<ffff000008707418>] tcp_v4_rcv+0xaf0/0xb58
> > [ 6709.000077] [<ffff0000086df784>] ip_local_deliver_finish+0x10c/0x254
> > [ 6709.006419] [<ffff0000086dfea4>] ip_local_deliver+0xf0/0xfc
> > [ 6709.011980] [<ffff0000086dfad4>] ip_rcv_finish+0x208/0x3a4
> > [ 6709.017454] [<ffff0000086e018c>] ip_rcv+0x2dc/0x3c8
> > [ 6709.022328] [<ffff000008692fc8>] __netif_receive_skb_core+0x2f8/0xa0c
> > [ 6709.028758] [<ffff000008696068>] __netif_receive_skb+0x38/0x84
> > [ 6709.034580] [<ffff00000869611c>] netif_receive_skb_internal+0x68/0xdc
> > [ 6709.041010] [<ffff000008696bc0>] napi_gro_receive+0xcc/0x1a8
> > [ 6709.046690] [<ffff0000014b0fc4>] nicvf_cq_intr_handler+0x59c/0x730 [nicvf]
> > [ 6709.053559] [<ffff0000014b1380>] nicvf_poll+0x38/0xb8 [nicvf]
> > [ 6709.059295] [<ffff000008697a6c>] net_rx_action+0x2f8/0x464
> > [ 6709.064771] [<ffff000008081824>] __do_softirq+0x11c/0x308
> > [ 6709.070164] [<ffff0000080d14e4>] irq_exit+0x12c/0x174
> > [ 6709.075206] [<ffff00000813101c>] __handle_domain_irq+0x78/0xc4
> > [ 6709.081027] [<ffff000008081608>] gic_handle_irq+0x94/0x190
> > [ 6709.086501] Exception stack(0xffff81000689fdf0 to 0xffff81000689ff20)
> > [ 6709.092929] fde0: 0000810ff2ec0000 ffff000008c10000
> > [ 6709.100747] fe00: ffff000008c70ef4 0000000000000001 0000000000000000 ffff810ffbad9b18
> > [ 6709.108565] fe20: ffff810ffbad9c70 ffff8100169d3800 ffff810006843ab0 ffff81000689fe80
> > [ 6709.116382] fe40: 0000000000000bd0 0000ffffdf979cd0 183f5913da192500 0000ffff8a254ce4
> > [ 6709.124200] fe60: 0000ffff8a254b78 0000aaab10339808 0000000000000000 0000ffff8a0c2a50
> > [ 6709.132018] fe80: 0000ffffdf979b10 ffff000008d6d450 ffff000008c10000 ffff000008d6d000
> > [ 6709.139836] fea0: 0000000000000054 ffff000008cd3dbc 0000000000000000 0000000000000000
> > [ 6709.147653] fec0: 0000000000000000 0000000000000000 0000000000000000 ffff81000689ff20
> > [ 6709.155471] fee0: ffff000008085240 ffff81000689ff20 ffff000008085244 0000000060000145
> > [ 6709.163289] ff00: ffff81000689ff10 ffff00000813f1e4 ffffffffffffffff ffff00000813f238
> > [ 6709.171107] [<ffff000008082eb4>] el1_irq+0xb4/0x140
> > [ 6709.175976] [<ffff000008085244>] arch_cpu_idle+0x44/0x11c
> > [ 6709.181368] [<ffff0000087bf3b8>] default_idle_call+0x20/0x30
> > [ 6709.187020] [<ffff000008116d50>] do_idle+0x158/0x1e4
> > [ 6709.191973] [<ffff000008116ff4>] cpu_startup_entry+0x2c/0x30
> > [ 6709.197624] [<ffff00000808e7cc>] secondary_start_kernel+0x13c/0x160
> > [ 6709.203878] [<0000000001bc71c4>] 0x1bc71c4
> > [ 6709.207967] Code: bad PC value
> > [ 6709.211061] SMP: stopping secondary CPUs
> > [ 6709.218830] Starting crashdump kernel...
> > [ 6709.222749] Bye!
> > ---<-snip>---
> >
> > Signed-off-by: Vadim Lomovtsev <vlomovts-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > ---
> > net/sunrpc/svcsock.c | 24 ++++++++++++++++++------
> > 1 file changed, 18 insertions(+), 6 deletions(-)
> >
> > diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
> > index 2b720fa..b6496f3 100644
> > --- a/net/sunrpc/svcsock.c
> > +++ b/net/sunrpc/svcsock.c
> > @@ -421,7 +421,9 @@ static void svc_data_ready(struct sock *sk)
> > dprintk("svc: socket %p(inet %p), busy=%d\n",
> > svsk, sk,
> > test_bit(XPT_BUSY, &svsk->sk_xprt.xpt_flags));
> > - svsk->sk_odata(sk);
> > + rmb();
> > + if (svsk->sk_odata)
> > + svsk->sk_odata(sk);
>
> I'm not convinced these checks will really do much, though you might
> need the rmb() calls there...see below...
>
> > if (!test_and_set_bit(XPT_DATA, &svsk->sk_xprt.xpt_flags))
> > svc_xprt_enqueue(&svsk->sk_xprt);
> > }
> > @@ -437,7 +439,9 @@ static void svc_write_space(struct sock *sk)
> > if (svsk) {
> > dprintk("svc: socket %p(inet %p), write_space busy=%d\n",
> > svsk, sk, test_bit(XPT_BUSY, &svsk->sk_xprt.xpt_flags));
> > - svsk->sk_owspace(sk);
> > + rmb();
> > + if (svsk->sk_owspace)
> > + svsk->sk_owspace(sk);
> > svc_xprt_enqueue(&svsk->sk_xprt);
> > }
> > }
> > @@ -760,8 +764,12 @@ static void svc_tcp_listen_data_ready(struct sock *sk)
> > dprintk("svc: socket %p TCP (listen) state change %d\n",
> > sk, sk->sk_state);
> >
> > - if (svsk)
> > - svsk->sk_odata(sk);
> > + if (svsk) {
> > + rmb();
> > + if (svsk->sk_odata)
> > + svsk->sk_odata(sk);
> > + }
> > +
> > /*
> > * This callback may called twice when a new connection
> > * is established as a child socket inherits everything
> > @@ -794,7 +802,10 @@ static void svc_tcp_state_change(struct sock *sk)
> > if (!svsk)
> > printk("svc: socket %p: no user data\n", sk);
> > else {
> > - svsk->sk_ostate(sk);
> > + rmb();
> > + if (svsk->sk_ostate)
> > + svsk->sk_ostate(sk);
> > +
> > if (sk->sk_state != TCP_ESTABLISHED) {
> > set_bit(XPT_CLOSE, &svsk->sk_xprt.xpt_flags);
> > svc_xprt_enqueue(&svsk->sk_xprt);
> > @@ -1381,12 +1392,13 @@ static struct svc_sock *svc_setup_socket(struct svc_serv *serv,
> > return ERR_PTR(err);
> > }
> >
> > - inet->sk_user_data = svsk;
> > svsk->sk_sock = sock;
> > svsk->sk_sk = inet;
> > svsk->sk_ostate = inet->sk_state_change;
> > svsk->sk_odata = inet->sk_data_ready;
> > svsk->sk_owspace = inet->sk_write_space;
> > + wmb();
> > + inet->sk_user_data = svsk;
> >
> > /* Initialize the socket */
> > if (sock->type == SOCK_DGRAM)
>
> Since we always check whether svsk is NULL before calling the functions,
> then the above hunk and maybe the rmb() calls might be all that's needed
> here. I don't see how we'd ever get a sk_odata value (for instance)
> that's NULL with the above change.
Thanks for feed-back. I'll update patch then, run test cycle once again
(hope couple days would be enough) and re-send v2 to review.
>
> --
> Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
WBR,
Vadim
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [iproute PATCH v2 1/7] ipntable: Make sure filter.name is NULL-terminated
From: Phil Sutter @ 2017-08-18 10:52 UTC (permalink / raw)
To: David Laight; +Cc: Stephen Hemminger, netdev@vger.kernel.org
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DD005A7B4@AcuExch.aculab.com>
Hi David,
On Fri, Aug 18, 2017 at 09:19:16AM +0000, David Laight wrote:
> From: Phil Sutter
> > Sent: 17 August 2017 18:09
> > To: Stephen Hemminger
> > Cc: netdev@vger.kernel.org
> > Subject: [iproute PATCH v2 1/7] ipntable: Make sure filter.name is NULL-terminated
> >
> > Signed-off-by: Phil Sutter <phil@nwl.cc>
> > ---
> > ip/ipntable.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/ip/ipntable.c b/ip/ipntable.c
> > index 879626ee4f491..7be1f04d33d90 100644
> > --- a/ip/ipntable.c
> > +++ b/ip/ipntable.c
> > @@ -633,7 +633,8 @@ static int ipntable_show(int argc, char **argv)
> > } else if (strcmp(*argv, "name") == 0) {
> > NEXT_ARG();
> >
> > - strncpy(filter.name, *argv, sizeof(filter.name));
> > + strncpy(filter.name, *argv, sizeof(filter.name) - 1);
> > + filter.name[sizeof(filter.name) - 1] = '\0';
>
> Why not check for overflow instead?
> if (filter.name[sizeof(filter.name) - 1])
> usage("filer name too long");
sizeof(filter.name) is 1024, which is maybe a bit over the top for
something a user would input. So I found a better way avoiding all this
at once: I made filter.name a const char *, then just assigned *argv to
it. This should be safe since rtnl_dump_filter() and therefore
print_ntable() callback is called from inside ipntable_show() so *argv
is not accessed outside of it's scope.
What do you think?
Thanks, Phil
^ 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