* [PATCHv2] netem: Segment GSO packets on enqueue.
From: Neil Horman @ 2016-04-28 20:09 UTC (permalink / raw)
To: netdev; +Cc: Neil Horman, Jamal Hadi Salim, David S. Miller, netem,
eric.dumazet
In-Reply-To: <1461692618-21333-1-git-send-email-nhorman@tuxdriver.com>
This was recently reported to me, and reproduced on the latest net kernel, when
attempting to run netperf from a host that had a netem qdisc attached to the
egress interface:
[ 788.073771] ------------[ cut here ]------------
[ 788.096716] WARNING: at net/core/dev.c:2253 skb_warn_bad_offload+0xcd/0xda()
[ 788.129521] bnx2: caps=(0x00000001801949b3, 0x0000000000000000) len=2962
data_len=0 gso_size=1448 gso_type=1 ip_summed=3
[ 788.182150] Modules linked in: sch_netem kvm_amd kvm crc32_pclmul ipmi_ssif
ghash_clmulni_intel sp5100_tco amd64_edac_mod aesni_intel lrw gf128mul
glue_helper ablk_helper edac_mce_amd cryptd pcspkr sg edac_core hpilo ipmi_si
i2c_piix4 k10temp fam15h_power hpwdt ipmi_msghandler shpchp acpi_power_meter
pcc_cpufreq nfsd auth_rpcgss nfs_acl lockd grace sunrpc ip_tables xfs libcrc32c
sd_mod crc_t10dif crct10dif_generic mgag200 syscopyarea sysfillrect sysimgblt
i2c_algo_bit drm_kms_helper ahci ata_generic pata_acpi ttm libahci
crct10dif_pclmul pata_atiixp tg3 libata crct10dif_common drm crc32c_intel ptp
serio_raw bnx2 r8169 hpsa pps_core i2c_core mii dm_mirror dm_region_hash dm_log
dm_mod
[ 788.465294] CPU: 16 PID: 0 Comm: swapper/16 Tainted: G W
------------ 3.10.0-327.el7.x86_64 #1
[ 788.511521] Hardware name: HP ProLiant DL385p Gen8, BIOS A28 12/17/2012
[ 788.542260] ffff880437c036b8 f7afc56532a53db9 ffff880437c03670
ffffffff816351f1
[ 788.576332] ffff880437c036a8 ffffffff8107b200 ffff880633e74200
ffff880231674000
[ 788.611943] 0000000000000001 0000000000000003 0000000000000000
ffff880437c03710
[ 788.647241] Call Trace:
[ 788.658817] <IRQ> [<ffffffff816351f1>] dump_stack+0x19/0x1b
[ 788.686193] [<ffffffff8107b200>] warn_slowpath_common+0x70/0xb0
[ 788.713803] [<ffffffff8107b29c>] warn_slowpath_fmt+0x5c/0x80
[ 788.741314] [<ffffffff812f92f3>] ? ___ratelimit+0x93/0x100
[ 788.767018] [<ffffffff81637f49>] skb_warn_bad_offload+0xcd/0xda
[ 788.796117] [<ffffffff8152950c>] skb_checksum_help+0x17c/0x190
[ 788.823392] [<ffffffffa01463a1>] netem_enqueue+0x741/0x7c0 [sch_netem]
[ 788.854487] [<ffffffff8152cb58>] dev_queue_xmit+0x2a8/0x570
[ 788.880870] [<ffffffff8156ae1d>] ip_finish_output+0x53d/0x7d0
...
The problem occurs because netem is not prepared to handle GSO packets (as it
uses skb_checksum_help in its enqueue path, which cannot manipulate these
frames).
The solution I think is to simply segment the skb in a simmilar fashion to the
way we do in __dev_queue_xmit (via validate_xmit_skb), except here we always
segment, instead of only when the interface needs us to do it. This allows
netem to properly drop/mangle/pass/etc the correct percentages of frames as per
its qdisc configuration, and avoid failing its checksum operations
tested successfully by myself on the latest net kernel, to whcih this applies
---
Change Notes:
V2) As per request from Eric Dumazet, I rewrote this to limit the need to
segment the skb. Instead of doing so unilaterally, we no only do so now when the
netem qdisc requires determines that a packet must be corrupted, thus avoiding
the failure in skb_checksum_help. This still leaves open concerns with
statistical measurements made on GSO packets being dropped or reordered (i.e.
they are counted as a single packet rather than multiple packets), but I'd
rather fix the immediate problem before we go rewriting everything to fix that
larger issue.
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
CC: Jamal Hadi Salim <jhs@mojatatu.com>
CC: "David S. Miller" <davem@davemloft.net>
CC: netem@lists.linux-foundation.org
CC: eric.dumazet@gmail.com
---
net/sched/sch_netem.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 48 insertions(+), 3 deletions(-)
diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
index 9640bb3..7cde5d3 100644
--- a/net/sched/sch_netem.c
+++ b/net/sched/sch_netem.c
@@ -395,6 +395,25 @@ static void tfifo_enqueue(struct sk_buff *nskb, struct Qdisc *sch)
sch->q.qlen++;
}
+/* netem can't properly corrupt a megapacket (like we get from GSO), so instead
+ * when we statistically choose to corrupt one, we instead segment it, returning
+ * the first packet to be corrupted, and re-enqueue the remaining frames
+ */
+static struct sk_buff* netem_segment(struct sk_buff *skb, struct Qdisc *sch)
+{
+ struct sk_buff *segs;
+ netdev_features_t features = netif_skb_features(skb);
+
+ segs = skb_gso_segment(skb, features & ~NETIF_F_GSO_MASK);
+
+ if (IS_ERR_OR_NULL(segs)) {
+ qdisc_reshape_fail(skb, sch);
+ return NULL;
+ }
+ consume_skb(skb);
+ return segs;
+}
+
/*
* Insert one skb into qdisc.
* Note: parent depends on return value to account for queue length.
@@ -407,7 +426,9 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch)
/* We don't fill cb now as skb_unshare() may invalidate it */
struct netem_skb_cb *cb;
struct sk_buff *skb2;
+ struct sk_buff *segs = NULL;
int count = 1;
+ int rc = NET_XMIT_SUCCESS;
/* Random duplication */
if (q->duplicate && q->duplicate >= get_crandom(&q->dup_cor))
@@ -453,10 +474,22 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch)
* do it now in software before we mangle it.
*/
if (q->corrupt && q->corrupt >= get_crandom(&q->corrupt_cor)) {
+ if (skb_is_gso(skb)) {
+ segs = netem_segment(skb, sch);
+ if (!segs)
+ return NET_XMIT_DROP;
+ } else
+ segs = skb;
+
+ skb = segs;
+ segs = segs->next;
+
if (!(skb = skb_unshare(skb, GFP_ATOMIC)) ||
(skb->ip_summed == CHECKSUM_PARTIAL &&
- skb_checksum_help(skb)))
- return qdisc_drop(skb, sch);
+ skb_checksum_help(skb))) {
+ rc = qdisc_drop(skb, sch);
+ goto finish_segs;
+ }
skb->data[prandom_u32() % skb_headlen(skb)] ^=
1<<(prandom_u32() % 8);
@@ -516,7 +549,19 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch)
sch->qstats.requeues++;
}
- return NET_XMIT_SUCCESS;
+finish_segs:
+ while (segs) {
+ skb2 = segs->next;
+ segs->next = NULL;
+ qdisc_skb_cb(segs)->pkt_len = segs->len;
+ rc = qdisc_enqueue(segs, sch);
+ if (rc != NET_XMIT_SUCCESS) {
+ if (net_xmit_drop_count(rc))
+ qdisc_qstats_drop(sch);
+ }
+ segs = skb2;
+ }
+ return rc;
}
static unsigned int netem_drop(struct Qdisc *sch)
--
2.5.5
^ permalink raw reply related
* [PATCH v3 net-next 2/2] net: ethernet: enc28j60: add device tree support
From: Michael Heimpold @ 2016-04-28 20:06 UTC (permalink / raw)
To: Jonathan Cameron, Andrew F . Davis, Mark Brown, netdev,
devicetree, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
Kumar Gala
Cc: Michael Heimpold
In-Reply-To: <1461873975-6368-1-git-send-email-mhei@heimpold.de>
The following patch adds the required match table for device tree support
(and while at, fix the indent). It's also possible to specify the
MAC address in the DT blob.
Also add the corresponding binding documentation file.
Signed-off-by: Michael Heimpold <mhei@heimpold.de>
---
Changelog:
v3: * renamed and improved binding documentation as
suggested by Rob Herring
v2: * took care of Arnd Bergmann's review comments
- allow to specify MAC address via DT
- unconditionally define DT id table
* increased the driver version minor number
* driver author's email address bounces, removed from address list
v1: * Initial submission
.../devicetree/bindings/net/microchip,enc28j60.txt | 59 ++++++++++++++++++++++
drivers/net/ethernet/microchip/enc28j60.c | 20 ++++++--
2 files changed, 76 insertions(+), 3 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/microchip,enc28j60.txt
diff --git a/Documentation/devicetree/bindings/net/microchip,enc28j60.txt b/Documentation/devicetree/bindings/net/microchip,enc28j60.txt
new file mode 100644
index 0000000..1dc3bc7
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/microchip,enc28j60.txt
@@ -0,0 +1,59 @@
+* Microchip ENC28J60
+
+This is a standalone 10 MBit ethernet controller with SPI interface.
+
+For each device connected to a SPI bus, define a child node within
+the SPI master node.
+
+Required properties:
+- compatible: Should be "microchip,enc28j60"
+- reg: Specify the SPI chip select the ENC28J60 is wired to
+- interrupt-parent: Specify the phandle of the source interrupt, see interrupt
+ binding documentation for details. Usually this is the GPIO bank
+ the interrupt line is wired to.
+- interrupts: Specify the interrupt index within the interrupt controller (referred
+ to above in interrupt-parent) and interrupt type. The ENC28J60 natively
+ generates falling edge interrupts, however, additional board logic
+ might invert the signal.
+- pinctrl-names: List of assigned state names, see pinctrl binding documentation.
+- pinctrl-0: List of phandles to configure the GPIO pin used as interrupt line,
+ see also generic and your platform specific pinctrl binding
+ documentation.
+
+Optional properties:
+- spi-max-frequency: Maximum frequency of the SPI bus when accessing the ENC28J60.
+ According to the ENC28J80 datasheet, the chip allows a maximum of 20 MHz, however,
+ board designs may need to limit this value.
+- local-mac-address: See ethernet.txt in the same directory.
+
+
+Example (for NXP i.MX28 with pin control stuff for GPIO irq):
+
+ ssp2: ssp@80014000 {
+ compatible = "fsl,imx28-spi";
+ pinctrl-names = "default";
+ pinctrl-0 = <&spi2_pins_b &spi2_sck_cfg>;
+ status = "okay";
+
+ enc28j60: ethernet@0 {
+ compatible = "microchip,enc28j60";
+ pinctrl-names = "default";
+ pinctrl-0 = <&enc28j60_pins>;
+ reg = <0>;
+ interrupt-parent = <&gpio3>;
+ interrupts = <3 IRQ_TYPE_EDGE_FALLING>;
+ spi-max-frequency = <12000000>;
+ };
+ };
+
+ pinctrl@80018000 {
+ enc28j60_pins: enc28j60_pins@0 {
+ reg = <0>;
+ fsl,pinmux-ids = <
+ MX28_PAD_AUART0_RTS__GPIO_3_3 /* Interrupt */
+ >;
+ fsl,drive-strength = <MXS_DRIVE_4mA>;
+ fsl,voltage = <MXS_VOLTAGE_HIGH>;
+ fsl,pull-up = <MXS_PULL_DISABLE>;
+ };
+ };
diff --git a/drivers/net/ethernet/microchip/enc28j60.c b/drivers/net/ethernet/microchip/enc28j60.c
index b723622..7066954 100644
--- a/drivers/net/ethernet/microchip/enc28j60.c
+++ b/drivers/net/ethernet/microchip/enc28j60.c
@@ -28,11 +28,12 @@
#include <linux/skbuff.h>
#include <linux/delay.h>
#include <linux/spi/spi.h>
+#include <linux/of_net.h>
#include "enc28j60_hw.h"
#define DRV_NAME "enc28j60"
-#define DRV_VERSION "1.01"
+#define DRV_VERSION "1.02"
#define SPI_OPLEN 1
@@ -1548,6 +1549,7 @@ static int enc28j60_probe(struct spi_device *spi)
{
struct net_device *dev;
struct enc28j60_net *priv;
+ const void *macaddr;
int ret = 0;
if (netif_msg_drv(&debug))
@@ -1579,7 +1581,12 @@ static int enc28j60_probe(struct spi_device *spi)
ret = -EIO;
goto error_irq;
}
- eth_hw_addr_random(dev);
+
+ macaddr = of_get_mac_address(spi->dev.of_node);
+ if (macaddr)
+ ether_addr_copy(dev->dev_addr, macaddr);
+ else
+ eth_hw_addr_random(dev);
enc28j60_set_hw_macaddr(dev);
/* Board setup must set the relevant edge trigger type;
@@ -1634,9 +1641,16 @@ static int enc28j60_remove(struct spi_device *spi)
return 0;
}
+static const struct of_device_id enc28j60_dt_ids[] = {
+ { .compatible = "microchip,enc28j60" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, enc28j60_dt_ids);
+
static struct spi_driver enc28j60_driver = {
.driver = {
- .name = DRV_NAME,
+ .name = DRV_NAME,
+ .of_match_table = enc28j60_dt_ids,
},
.probe = enc28j60_probe,
.remove = enc28j60_remove,
--
2.5.0
^ permalink raw reply related
* [PATCH v3 net-next 1/2] net: ethernet: enc28j60: support half-duplex SPI controllers
From: Michael Heimpold @ 2016-04-28 20:06 UTC (permalink / raw)
To: Jonathan Cameron, Andrew F . Davis, Mark Brown, netdev; +Cc: Michael Heimpold
In-Reply-To: <1461873975-6368-1-git-send-email-mhei@heimpold.de>
The current spi_read_buf function fails on SPI host masters which
are only half-duplex capable. Splitting the Tx and Rx part solves
this issue.
Tested on Raspberry Pi (full duplex) and I2SE Duckbill (half duplex).
Signed-off-by: Michael Heimpold <mhei@heimpold.de>
---
Changelog:
v3: - no changes
v2: - no changes
v1: * Initial submission
drivers/net/ethernet/microchip/enc28j60.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/microchip/enc28j60.c b/drivers/net/ethernet/microchip/enc28j60.c
index 86ea17e..b723622 100644
--- a/drivers/net/ethernet/microchip/enc28j60.c
+++ b/drivers/net/ethernet/microchip/enc28j60.c
@@ -89,22 +89,26 @@ spi_read_buf(struct enc28j60_net *priv, int len, u8 *data)
{
u8 *rx_buf = priv->spi_transfer_buf + 4;
u8 *tx_buf = priv->spi_transfer_buf;
- struct spi_transfer t = {
+ struct spi_transfer tx = {
.tx_buf = tx_buf,
+ .len = SPI_OPLEN,
+ };
+ struct spi_transfer rx = {
.rx_buf = rx_buf,
- .len = SPI_OPLEN + len,
+ .len = len,
};
struct spi_message msg;
int ret;
tx_buf[0] = ENC28J60_READ_BUF_MEM;
- tx_buf[1] = tx_buf[2] = tx_buf[3] = 0; /* don't care */
spi_message_init(&msg);
- spi_message_add_tail(&t, &msg);
+ spi_message_add_tail(&tx, &msg);
+ spi_message_add_tail(&rx, &msg);
+
ret = spi_sync(priv->spi, &msg);
if (ret == 0) {
- memcpy(data, &rx_buf[SPI_OPLEN], len);
+ memcpy(data, rx_buf, len);
ret = msg.status;
}
if (ret && netif_msg_drv(priv))
--
2.5.0
^ permalink raw reply related
* Re: [PATCH v2 net-next 0/2] tcp: simplify ack tx timestamps
From: David Miller @ 2016-04-28 20:06 UTC (permalink / raw)
To: soheil.kdev; +Cc: netdev, kafai, willemb, edumazet, ycheng, ncardwell, soheil
In-Reply-To: <1461814741-848-1-git-send-email-soheil.kdev@gmail.com>
From: Soheil Hassas Yeganeh <soheil.kdev@gmail.com>
Date: Wed, 27 Apr 2016 23:38:59 -0400
> From: Soheil Hassas Yeganeh <soheil@google.com>
>
> v2:
> - Fully remove SKBTX_ACK_TSTAMP, as suggested by Willem de Bruijn.
>
> This patch series aims at removing redundant checks and fields
> for ack timestamps for TCP.
Series applied, thank you.
^ permalink raw reply
* [PATCH v3 net-next 0/2] net: ethernet: enc28j60: small improvements
From: Michael Heimpold @ 2016-04-28 20:06 UTC (permalink / raw)
To: Jonathan Cameron, Andrew F . Davis, Mark Brown, netdev; +Cc: Michael Heimpold
This series of two patches adds the following improvements to the driver:
1) Rework the central SPI read function so that it is compatible with
SPI masters which only support half duplex transfers.
2) Add a device tree binding for the driver.
Changelog:
v3: * renamed and improved binding documentation as
suggested by Rob Herring
v2: * took care of Arnd Bergmann's review comments
- allow to specify MAC address via DT
- unconditionally define DT id table
* increased the driver version minor number
* driver author's email address bounces, removed from address list
v1: * Initial submission
Michael Heimpold (2):
net: ethernet: enc28j60: support half-duplex SPI controllers
net: ethernet: enc28j60: add device tree support
.../devicetree/bindings/net/microchip,enc28j60.txt | 59 ++++++++++++++++++++++
drivers/net/ethernet/microchip/enc28j60.c | 34 ++++++++++---
2 files changed, 85 insertions(+), 8 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/microchip,enc28j60.txt
--
2.5.0
^ permalink raw reply
* Re: [PATCH v2] net: macb: do not scan PHYs manually
From: Florian Fainelli @ 2016-04-28 20:03 UTC (permalink / raw)
To: Andrew Lunn, Nathan Sullivan
Cc: Nicolas Ferre, netdev, linux-kernel, Alexandre Belloni
In-Reply-To: <20160428185932.GU29024@lunn.ch>
On 28/04/16 11:59, Andrew Lunn wrote:
> On Thu, Apr 28, 2016 at 01:55:27PM -0500, Nathan Sullivan wrote:
>> On Thu, Apr 28, 2016 at 08:43:03PM +0200, Andrew Lunn wrote:
>>>> I agree that is a valid fix for AT91, however it won't solve our problem, since
>>>> we have no children on the second ethernet MAC in our devices' device trees. I'm
>>>> starting to feel like our second MAC shouldn't even really register the MDIO bus
>>>> since it isn't being used - maybe adding a DT property to not have a bus is a
>>>> better option?
>>>
>>> status = "disabled"
>>>
>>> would be the unusual way.
>>>
>>> Andrew
>>
>> Oh, sorry, I meant we use both MACs on Zynq, however the PHYs are on the MDIO
>> bus of the first MAC. So, the second MAC is used for ethernet but not for MDIO,
>> and so it does not have any PHYs under its DT node. It would be nice if there
>> were a way to tell macb not to bother with MDIO for the second MAC, since that's
>> handled by the first MAC.
>
> Yes, exactly, add support for status = "disabled" in the mdio node.
Something like that, just so we do not have to sprinkle tests all other
the place:
diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
index b622b33dbf93..2f497790be1b 100644
--- a/drivers/of/of_mdio.c
+++ b/drivers/of/of_mdio.c
@@ -209,6 +209,10 @@ int of_mdiobus_register(struct mii_bus *mdio,
struct device_node *np)
bool scanphys = false;
int addr, rc;
+ /* Do not continue if the node is disabled */
+ if (!of_device_is_available(np))
+ return -EINVAL;
+
/* Mask out all PHYs from auto probing. Instead the PHYs listed in
* the device tree are populated after the bus has been
registered */
mdio->phy_mask = ~0;
>
>> I guess a good longer-term solution to all these problems would be to treat the
>> MAC and MDIO as seperate devices, like davinci seems to be doing.
>
> A few others do this as well, e.g. most Marvell devices.
Sometimes the MDIO registers are intertwinned with the Ethernet MAC
register space, which is something you can solve by handing just the
relevant portion of the MDIO register space to a separate driver (though
you need to watch out for two drivers calling request_mem_region on the
same register space).
--
Florian
^ permalink raw reply related
* Re: [PATCH] net: fix net_gso_ok for new GSO types.
From: David Miller @ 2016-04-28 19:53 UTC (permalink / raw)
To: marcelo.leitner; +Cc: netdev
In-Reply-To: <3d1dadb911fa4ebacf1ca5ad298a5c696ed261a6.1461607119.git.marcelo.leitner@gmail.com>
From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Date: Mon, 25 Apr 2016 15:13:17 -0300
> Fix casting in net_gso_ok. Otherwise the shift on
> gso_type << NETIF_F_GSO_SHIFT may hit the 32th bit and make it look like
> a INT_MIN, which is then promoted from signed to uint64 which is
> 0xffffffff80000000, resulting in wrong behavior when it is and'ed with
> the feature itself, as in:
...
> So that this:
> return (features & feature) == feature;
> Actually works on more bits than expected and invalid ones.
>
> Fix is to promote it earlier.
>
> Issue noted while rebasing SCTP GSO patch but posting separetely as
> someone else may experience this meanwhile.
>
> Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Great catch, applied, thanks Marcelo.
^ permalink raw reply
* Re: [PATCH] net: ethernet: davinci_emac: Fix devioctl while in fixed link
From: David Miller @ 2016-04-28 19:52 UTC (permalink / raw)
To: narmstrong
Cc: andrew, thomas.lendacky, mugunthanvnm, netdev, linux-kernel,
b.hutchman
In-Reply-To: <1461606098-20057-1-git-send-email-narmstrong@baylibre.com>
From: Neil Armstrong <narmstrong@baylibre.com>
Date: Mon, 25 Apr 2016 19:41:38 +0200
> When configured in fixed link, the DaVinci emac driver sets the
> priv->phydev to NULL and further ioctl calls to the phy_mii_ioctl()
> causes the kernel to crash.
>
> Cc: Brian Hutchinson <b.hutchman@gmail.com>
> Fixes: 1bb6aa56bb38 ("net: davinci_emac: Add support for fixed-link PHY")
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
Applied, thanks Neil.
^ permalink raw reply
* Re: [PATCH] net: davinci_mdio: Set of_node in the mdio bus
From: David Miller @ 2016-04-28 19:44 UTC (permalink / raw)
To: Linux.HWI
Cc: linux-kernel, netdev, Grygorii.Strashko, jay.schroeder,
ben.mccauley
In-Reply-To: <1461595571-11438-1-git-send-email-Linux.HWI@garmin.com>
From: "J.D. Schroeder" <Linux.HWI@garmin.com>
Date: Mon, 25 Apr 2016 09:46:11 -0500
> From: "J.D. Schroeder" <jay.schroeder@garmin.com>
>
> Assigns the of_node from the platform device to the of_node of the
> mdio bus so that it can be used in the mdio driver to properly match
> a bus in the DT with a phandle in of_mdio_find_bus().
>
> Signed-off-by: J.D. Schroeder <jay.schroeder@garmin.com>
> Signed-off-by: Ben McCauley <ben.mccauley@garmin.com>
> ---
> drivers/net/ethernet/ti/davinci_mdio.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/ethernet/ti/davinci_mdio.c b/drivers/net/ethernet/ti/davinci_mdio.c
> index 4e7c9b9..b5e5f37 100644
> --- a/drivers/net/ethernet/ti/davinci_mdio.c
> +++ b/drivers/net/ethernet/ti/davinci_mdio.c
> @@ -343,6 +343,7 @@ static int davinci_mdio_probe(struct platform_device *pdev)
> if (davinci_mdio_probe_dt(&data->pdata, pdev))
> data->pdata = default_pdata;
> snprintf(data->bus->id, MII_BUS_ID_SIZE, "%s", pdev->name);
> + data->bus->dev.of_node = dev->of_node;
> } else {
> data->pdata = pdata ? (*pdata) : default_pdata;
> snprintf(data->bus->id, MII_BUS_ID_SIZE, "%s-%x",
You can't do this.
First of all, of_node objects are reference counted. So even if this was a
legal thing to do you would have to drop the reference to the existing of_node
pointer and gain a reference to dev->of_node.
But even more importantly, it is the job of the bus driver to set that
bus->dev.of_node correctly, you should never override it in a driver like
this.
I'm not applying this, sorry.
^ permalink raw reply
* Re: [PATCH] net: l2tp: fix reversed udp6 checksum flags
From: Wang Shanker @ 2016-04-28 19:25 UTC (permalink / raw)
To: James Chapman; +Cc: netdev, Tom Herbert, David S. Miller
In-Reply-To: <CAEwTi7SejvT35H0S0nBArEZFwoFAcCJ5twTKUmTxHcNx8az7Rg@mail.gmail.com>
I think this is a logic error, rather than a change to the default
UDP checksum setting. As expected, take rx for example, the flag
`L2TP_ATTR_UDP_ZERO_CSUM6_RX` is not set by default, and udp6
checksum will be checked by default. The fact is that, not setting
`L2TP_ATTR_UDP_ZERO_CSUM6_RX` leads to ignoring udp6 checksum. Such
a behavior does not correspond to the name
“L2TP_ATTR_UDP_ZERO_CSUM6_RX”. As a result, I call it a logic error.
> 在 2016年4月29日，02:46，James Chapman <jchapman@katalix.com> 写道：
>
> Some additional background on this: Wang found this when configuring
> l2tp tunnels using "ip l2tp" between two systems and then one system
> was upgraded. The tunnel failed to pass data because one side had UDP
> checksums enabled and the other now had them disabled. It seems kernel
> changes related to UDP checksums resulted in a change to the default
> UDP checksum setting for L2TP tunnels when using IPv6. Unfortunately,
> iproute2 doesn't let the user configure L2TP UDP checksum settings, so
> without this fix, some users may see problems depending on the kernel
> version differences on the L2TP peers. One for stable?
>
> Acked-by: James Chapman <jchapman@katalix.com>
>
> On 28 April 2016 at 18:29, Wang Shanker <shankerwangmiao@gmail.com> wrote:
>> This patch fixes a bug which causes the behavior of whether to ignore
>> udp6 checksum of udp6 encapsulated l2tp tunnel contrary to what
>> userspace program requests.
>>
>> When the flag `L2TP_ATTR_UDP_ZERO_CSUM6_RX` is set by userspace, it is
>> expected that udp6 checksums of received packets of the l2tp tunnel
>> to create should be ignored. In `l2tp_netlink.c`:
>> `l2tp_nl_cmd_tunnel_create()`, `cfg.udp6_zero_rx_checksums` is set
>> according to the flag, and then passed to `l2tp_core.c`:
>> `l2tp_tunnel_create()` and then `l2tp_tunnel_sock_create()`. In
>> `l2tp_tunnel_sock_create()`, `udp_conf.use_udp6_rx_checksums` is set
>> the same to `cfg.udp6_zero_rx_checksums`. However, if we want the
>> checksum to be ignored, `udp_conf.use_udp6_rx_checksums` should be set
>> to `false`, i.e. be set to the contrary. Similarly, the same should be
>> done to `udp_conf.use_udp6_tx_checksums`.
>>
>> Signed-off-by: Miao Wang <shankerwangmiao@gmail.com>
>> ---
>> net/l2tp/l2tp_core.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
>> index afca2eb..6edfa99 100644
>> --- a/net/l2tp/l2tp_core.c
>> +++ b/net/l2tp/l2tp_core.c
>> @@ -1376,9 +1376,9 @@ static int l2tp_tunnel_sock_create(struct net *net,
>> memcpy(&udp_conf.peer_ip6, cfg->peer_ip6,
>> sizeof(udp_conf.peer_ip6));
>> udp_conf.use_udp6_tx_checksums =
>> - cfg->udp6_zero_tx_checksums;
>> + ! cfg->udp6_zero_tx_checksums;
>> udp_conf.use_udp6_rx_checksums =
>> - cfg->udp6_zero_rx_checksums;
>> + ! cfg->udp6_zero_rx_checksums;
>> } else
>> #endif
>> {
>> --
>> 2.5.2
>>
^ permalink raw reply
* Re: [PATCH v2] net: macb: do not scan PHYs manually
From: Andrew Lunn @ 2016-04-28 18:59 UTC (permalink / raw)
To: Nathan Sullivan
Cc: Nicolas Ferre, netdev, linux-kernel, Florian Fainelli,
Alexandre Belloni
In-Reply-To: <20160428185527.GA8851@nathan3500-linux-VM>
On Thu, Apr 28, 2016 at 01:55:27PM -0500, Nathan Sullivan wrote:
> On Thu, Apr 28, 2016 at 08:43:03PM +0200, Andrew Lunn wrote:
> > > I agree that is a valid fix for AT91, however it won't solve our problem, since
> > > we have no children on the second ethernet MAC in our devices' device trees. I'm
> > > starting to feel like our second MAC shouldn't even really register the MDIO bus
> > > since it isn't being used - maybe adding a DT property to not have a bus is a
> > > better option?
> >
> > status = "disabled"
> >
> > would be the unusual way.
> >
> > Andrew
>
> Oh, sorry, I meant we use both MACs on Zynq, however the PHYs are on the MDIO
> bus of the first MAC. So, the second MAC is used for ethernet but not for MDIO,
> and so it does not have any PHYs under its DT node. It would be nice if there
> were a way to tell macb not to bother with MDIO for the second MAC, since that's
> handled by the first MAC.
Yes, exactly, add support for status = "disabled" in the mdio node.
> I guess a good longer-term solution to all these problems would be to treat the
> MAC and MDIO as seperate devices, like davinci seems to be doing.
A few others do this as well, e.g. most Marvell devices.
Andrew
^ permalink raw reply
* Re: [PATCH v2] net: macb: do not scan PHYs manually
From: Nathan Sullivan @ 2016-04-28 18:55 UTC (permalink / raw)
To: Andrew Lunn
Cc: Nicolas Ferre, netdev, linux-kernel, Florian Fainelli,
Alexandre Belloni
In-Reply-To: <20160428184303.GR29024@lunn.ch>
On Thu, Apr 28, 2016 at 08:43:03PM +0200, Andrew Lunn wrote:
> > I agree that is a valid fix for AT91, however it won't solve our problem, since
> > we have no children on the second ethernet MAC in our devices' device trees. I'm
> > starting to feel like our second MAC shouldn't even really register the MDIO bus
> > since it isn't being used - maybe adding a DT property to not have a bus is a
> > better option?
>
> status = "disabled"
>
> would be the unusual way.
>
> Andrew
Oh, sorry, I meant we use both MACs on Zynq, however the PHYs are on the MDIO
bus of the first MAC. So, the second MAC is used for ethernet but not for MDIO,
and so it does not have any PHYs under its DT node. It would be nice if there
were a way to tell macb not to bother with MDIO for the second MAC, since that's
handled by the first MAC.
I guess a good longer-term solution to all these problems would be to treat the
MAC and MDIO as seperate devices, like davinci seems to be doing.
^ permalink raw reply
* Re: [PATCH] net: l2tp: fix reversed udp6 checksum flags
From: James Chapman @ 2016-04-28 18:46 UTC (permalink / raw)
To: Wang Shanker; +Cc: netdev, Tom Herbert, David S. Miller
In-Reply-To: <9A325D67-FED1-4AF6-8ED0-6C93FFE8DC67@gmail.com>
Some additional background on this: Wang found this when configuring
l2tp tunnels using "ip l2tp" between two systems and then one system
was upgraded. The tunnel failed to pass data because one side had UDP
checksums enabled and the other now had them disabled. It seems kernel
changes related to UDP checksums resulted in a change to the default
UDP checksum setting for L2TP tunnels when using IPv6. Unfortunately,
iproute2 doesn't let the user configure L2TP UDP checksum settings, so
without this fix, some users may see problems depending on the kernel
version differences on the L2TP peers. One for stable?
Acked-by: James Chapman <jchapman@katalix.com>
On 28 April 2016 at 18:29, Wang Shanker <shankerwangmiao@gmail.com> wrote:
> This patch fixes a bug which causes the behavior of whether to ignore
> udp6 checksum of udp6 encapsulated l2tp tunnel contrary to what
> userspace program requests.
>
> When the flag `L2TP_ATTR_UDP_ZERO_CSUM6_RX` is set by userspace, it is
> expected that udp6 checksums of received packets of the l2tp tunnel
> to create should be ignored. In `l2tp_netlink.c`:
> `l2tp_nl_cmd_tunnel_create()`, `cfg.udp6_zero_rx_checksums` is set
> according to the flag, and then passed to `l2tp_core.c`:
> `l2tp_tunnel_create()` and then `l2tp_tunnel_sock_create()`. In
> `l2tp_tunnel_sock_create()`, `udp_conf.use_udp6_rx_checksums` is set
> the same to `cfg.udp6_zero_rx_checksums`. However, if we want the
> checksum to be ignored, `udp_conf.use_udp6_rx_checksums` should be set
> to `false`, i.e. be set to the contrary. Similarly, the same should be
> done to `udp_conf.use_udp6_tx_checksums`.
>
> Signed-off-by: Miao Wang <shankerwangmiao@gmail.com>
> ---
> net/l2tp/l2tp_core.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
> index afca2eb..6edfa99 100644
> --- a/net/l2tp/l2tp_core.c
> +++ b/net/l2tp/l2tp_core.c
> @@ -1376,9 +1376,9 @@ static int l2tp_tunnel_sock_create(struct net *net,
> memcpy(&udp_conf.peer_ip6, cfg->peer_ip6,
> sizeof(udp_conf.peer_ip6));
> udp_conf.use_udp6_tx_checksums =
> - cfg->udp6_zero_tx_checksums;
> + ! cfg->udp6_zero_tx_checksums;
> udp_conf.use_udp6_rx_checksums =
> - cfg->udp6_zero_rx_checksums;
> + ! cfg->udp6_zero_rx_checksums;
> } else
> #endif
> {
> --
> 2.5.2
>
^ permalink raw reply
* Re: [PATCH v2] net: macb: do not scan PHYs manually
From: Andrew Lunn @ 2016-04-28 18:43 UTC (permalink / raw)
To: Nathan Sullivan
Cc: Nicolas Ferre, netdev, linux-kernel, Florian Fainelli,
Alexandre Belloni
In-Reply-To: <20160428175619.GA8791@nathan3500-linux-VM>
> I agree that is a valid fix for AT91, however it won't solve our problem, since
> we have no children on the second ethernet MAC in our devices' device trees. I'm
> starting to feel like our second MAC shouldn't even really register the MDIO bus
> since it isn't being used - maybe adding a DT property to not have a bus is a
> better option?
status = "disabled"
would be the unusual way.
Andrew
^ permalink raw reply
* Re: [RFC 07/20] net: dsa: list ports in switch\\
From: Florian Fainelli @ 2016-04-28 18:29 UTC (permalink / raw)
To: Vivien Didelot, Andrew Lunn
Cc: netdev, linux-kernel, kernel, David S. Miller, Jiri Pirko
In-Reply-To: <871t5pr3zm.fsf@ketchup.mtl.sfl>
On 28/04/16 11:18, Vivien Didelot wrote:
> Florian Fainelli <f.fainelli@gmail.com> writes:
>
>> On 27/04/16 16:15, Andrew Lunn wrote:
>>> On Wed, Apr 27, 2016 at 06:30:04PM -0400, Vivien Didelot wrote:
>>>> List DSA port structures in their switch structure, so that drivers can
>>>> iterate on them to retrieve information such as their ports membership.
>>>
>>> And this would be so much easier using a plan array.
>>
>> Agreed, I do not see much value in doing this at the moment. Even if you
>> have unused ports in a switch, allocating an array is a small price to
>> pay compared to directly indexing by port number.
>>
>> NAK from me unless there is a compelling reason for doing so.
>
> The point of having a list is 1) get rid of the DSA_MAX_PORTS and have
> variable number of ports 2) lists make iteration easier with variable
> number of switchs/ports, e.g.:
You could get rid of the DSA_MAX_PORTS by asking switch drivers how many
ports they support and allocate that dynamically.
>
> dsa_tree_for_each_switch(dst, ds)
> dsa_switch_for_each_port(ds, dp)
> /* do something with the port */;
This is not more compact or efficient than an array walk, but at this
point this becoming preference over anything.
>
> Anyway, I'm writing a proposal for a new design of DSA, in order to
> support the D in DSA. That way, we'll avoid reviewing details of the
> implementation and have a big picture of the necessary API changes.
Quite frankly, I think your set of changes are submitted at a terrible
time, I would very much prefer to allow Andrew to complete his work on
re-designing the DSA layer to allow different kinds of switches, thus
allowing other people to support more HW in a blink of an eye, and
therefore allowing us all to get a clearer picture of what these little
switches are capable, rather than some patches that produce a lot of
churn with little documented benefits outside of the cross-chip operations.
Don't get me wrong, I think we should get to the point where you want us
to go, and work in that area is very much appreciated!
Thanks
--
Florian
^ permalink raw reply
* Re: pull-request: wireless-drivers 2016-04-25
From: David Miller @ 2016-04-28 18:23 UTC (permalink / raw)
To: kvalo; +Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <87a8kh3bvj.fsf@kamboji.qca.qualcomm.com>
From: Kalle Valo <kvalo@codeaurora.org>
Date: Mon, 25 Apr 2016 19:13:20 +0300
> few fixes for 4.6, more info in the signed tag below. I'm hoping this to
> be the final pull request for 4.6 but let's see how it goes. Please let
> me know if there are any problems.
Pulled, thanks.
^ permalink raw reply
* Re: [PATCH net] MAINTAINERS: net: update sfc maintainers
From: David Miller @ 2016-04-28 18:22 UTC (permalink / raw)
To: bkenward; +Cc: linux-kernel, netdev, ecree, sshah, linux-net-drivers
In-Reply-To: <571E48E4.4050106@solarflare.com>
From: Bert Kenward <bkenward@solarflare.com>
Date: Mon, 25 Apr 2016 17:42:12 +0100
> Add myself and Edward Cree as maintainers.
> Remove Shradha Shah, who is on extended leave.
>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Edward Cree <ecree@solarflare.com>
> Cc: Shradha Shah <sshah@solarflare.com>
> Signed-off-by: Bert Kenward <bkenward@solarflare.com>
Applied.
^ permalink raw reply
* Re: [PATCH net] sfc: disable RSS when unsupported
From: David Miller @ 2016-04-28 18:22 UTC (permalink / raw)
To: bkenward; +Cc: netdev, jcooper, linux-net-drivers
In-Reply-To: <571E3CE4.6010808@solarflare.com>
From: Bert Kenward <bkenward@solarflare.com>
Date: Mon, 25 Apr 2016 16:51:00 +0100
> From: Jon Cooper <jcooper@solarflare.com>
>
> When certain firmware variants are selected (via the sfboot utility) the
> SFC7000 and SFC8000 series NICs don't support RSS. The driver still
> tries (and fails) to insert filters with the RSS flag, and the NIC fails
> to pass traffic.
>
> When the firmware reports RSS_LIMITED suppress allocating a default RSS
> context. The absence of an RSS context is picked up in filter insertion
> and RSS flags are discarded.
>
> Signed-off-by: Bert Kenward <bkenward@solarflare.com>
Applied.
^ permalink raw reply
* Re: myri10ge: fix sleeping with bh disabled
From: David Miller @ 2016-04-28 18:21 UTC (permalink / raw)
To: sgruszka; +Cc: netdev, hykim
In-Reply-To: <20160425085918.GB2608@redhat.com>
From: Stanislaw Gruszka <sgruszka@redhat.com>
Date: Mon, 25 Apr 2016 10:59:19 +0200
> napi_disable() can not be called with bh disabled, move locking just
> around myri10ge_ss_lock_napi() .
>
> Patches fixes following bug:
>
> [ 114.278378] BUG: sleeping function called from invalid context at net/core/dev.c:4383
> <snip>
> [ 114.313712] Call Trace:
> [ 114.314943] [<ffffffff817010ce>] dump_stack+0x19/0x1b
> [ 114.317673] [<ffffffff810ce7f3>] __might_sleep+0x173/0x230
> [ 114.320566] [<ffffffff815b3117>] napi_disable+0x27/0x90
> [ 114.323254] [<ffffffffa01e437f>] myri10ge_close+0xbf/0x3f0 [myri10ge]
>
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
Applied.
^ permalink raw reply
* Re: [PATCH 30/41] Documentation: networking: fix spelling mistakes
From: David Miller @ 2016-04-28 18:21 UTC (permalink / raw)
To: eric; +Cc: linux-kernel, corbet, dsa, shm, brouer, ast, linux-doc, netdev
In-Reply-To: <1461566229-4717-4-git-send-email-eric@engestrom.ch>
From: Eric Engestrom <eric@engestrom.ch>
Date: Mon, 25 Apr 2016 07:36:56 +0100
> Signed-off-by: Eric Engestrom <eric@engestrom.ch>
Applied.
^ permalink raw reply
* Re: [PATCH net] RDMA/nes: don't leak skb if carrier down
From: David Miller @ 2016-04-28 18:20 UTC (permalink / raw)
To: fw-HFFVJYpyMKqzQB+pC5nmwQ
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1461529139-28582-1-git-send-email-fw-HFFVJYpyMKqzQB+pC5nmwQ@public.gmane.org>
From: Florian Westphal <fw-HFFVJYpyMKqzQB+pC5nmwQ@public.gmane.org>
Date: Sun, 24 Apr 2016 22:18:59 +0200
> Alternatively one could free the skb, OTOH I don't think this test is
> useful so just remove it.
>
> Cc: <linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
> Signed-off-by: Florian Westphal <fw-HFFVJYpyMKqzQB+pC5nmwQ@public.gmane.org>
> ---
> Noticed this while working on the TX_LOCKED removal.
Assuming Doug will take this.
--
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
* Re: [RFC 07/20] net: dsa: list ports in switch\\
From: Vivien Didelot @ 2016-04-28 18:18 UTC (permalink / raw)
To: Florian Fainelli, Andrew Lunn
Cc: netdev, linux-kernel, kernel, David S. Miller, Jiri Pirko
In-Reply-To: <572241BB.1070304@gmail.com>
Florian Fainelli <f.fainelli@gmail.com> writes:
> On 27/04/16 16:15, Andrew Lunn wrote:
>> On Wed, Apr 27, 2016 at 06:30:04PM -0400, Vivien Didelot wrote:
>>> List DSA port structures in their switch structure, so that drivers can
>>> iterate on them to retrieve information such as their ports membership.
>>
>> And this would be so much easier using a plan array.
>
> Agreed, I do not see much value in doing this at the moment. Even if you
> have unused ports in a switch, allocating an array is a small price to
> pay compared to directly indexing by port number.
>
> NAK from me unless there is a compelling reason for doing so.
The point of having a list is 1) get rid of the DSA_MAX_PORTS and have
variable number of ports 2) lists make iteration easier with variable
number of switchs/ports, e.g.:
dsa_tree_for_each_switch(dst, ds)
dsa_switch_for_each_port(ds, dp)
/* do something with the port */;
Anyway, I'm writing a proposal for a new design of DSA, in order to
support the D in DSA. That way, we'll avoid reviewing details of the
implementation and have a big picture of the necessary API changes.
Thanks,
Vivien
^ permalink raw reply
* Re: [PATCH net-next 0/6] net: make TCP preemptible
From: Eric Dumazet @ 2016-04-28 18:04 UTC (permalink / raw)
To: Alexei Starovoitov; +Cc: Eric Dumazet, David S . Miller, netdev
In-Reply-To: <20160428175154.GA83737@ast-mbp.thefacebook.com>
On Thu, 2016-04-28 at 10:51 -0700, Alexei Starovoitov wrote:
> got it. It applies to both v4 and v6, right?
You mean IPv6 ?
Well, you know, I decided to skip IPv6 and go to IPv8 anyway ;)
Yes, these changes apply for both IPv4 and IPv6 in the meantime.
> that would be awesome. I think map/reduce type jobs typically
> do large sendmsg, so it should help p99 latency.
Note that it should even help to send faster, since the ACK packets or
TCP Small Queue handlers are blocked until the sendmsg() was complete.
ACK packets usually open the window for more sends.
Many applications are using a loop and multiple sendmsg( ) with ~64KB
chunks to work around these latency issues.
^ permalink raw reply
* Re: [PATCH v2] net: macb: do not scan PHYs manually
From: Nathan Sullivan @ 2016-04-28 17:56 UTC (permalink / raw)
To: Andrew Lunn
Cc: Nicolas Ferre, netdev, linux-kernel, Florian Fainelli,
Alexandre Belloni
In-Reply-To: <20160428163207.GP29024@lunn.ch>
On Thu, Apr 28, 2016 at 06:32:07PM +0200, Andrew Lunn wrote:
> > Hmm, are AT91 platforms special in this regard? As far as I can tell, this
> > driver (macb) and Marvell PXA are the only ethernet drivers that call
> > mdiobus_scan directly, and PXA does it on a known address. I do see that there
> > are trees that use macb and don't have a phy listed, which is unfortunate.
>
> How it is supposed to work is that you do one of two things:
>
> 1) Your device tree does not have an mdio node. In this case, you call
> mdiobus_register() and it will perform a scan of the bus, and find the
> phys.
>
> 2) Your device tree does have an MDIO node, and you list your PHYs.
>
> Having an MDIO node and not listing the PHYs is broken...
>
> There are however, a few broken device trees around, and a few drivers
> have workarounds. e.g. davinci_mdio.c
>
> /* register the mii bus
> * Create PHYs from DT only in case if PHY child nodes are explicitly
> * defined to support backward compatibility with DTs which assume that
> * Davinci MDIO will always scan the bus for PHYs detection.
> */
> if (dev->of_node && of_get_child_count(dev->of_node)) {
> data->skip_scan = true;
> ret = of_mdiobus_register(data->bus, dev->of_node);
> } else {
> ret = mdiobus_register(data->bus);
> }
>
> You probably need to do the same for AT91, count the number of
> children, and it if it zero, fall back to the non-DT way. It would
> also be good to print a warning to get people to fix their device
> tree.
>
> Andrew
I agree that is a valid fix for AT91, however it won't solve our problem, since
we have no children on the second ethernet MAC in our devices' device trees. I'm
starting to feel like our second MAC shouldn't even really register the MDIO bus
since it isn't being used - maybe adding a DT property to not have a bus is a
better option?
^ permalink raw reply
* Re: [PATCH net-next 0/6] net: make TCP preemptible
From: Alexei Starovoitov @ 2016-04-28 17:51 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Eric Dumazet, David S . Miller, netdev
In-Reply-To: <1461865270.5535.109.camel@edumazet-glaptop3.roam.corp.google.com>
On Thu, Apr 28, 2016 at 10:41:10AM -0700, Eric Dumazet wrote:
> On Thu, 2016-04-28 at 10:23 -0700, Alexei Starovoitov wrote:
> > On Wed, Apr 27, 2016 at 10:25:46PM -0700, Eric Dumazet wrote:
> > > Most of TCP stack assumed it was running from BH handler.
> > >
> > > This is great for most things, as TCP behavior is very sensitive
> > > to scheduling artifacts.
> > >
> > > However, the prequeue and backlog processing are problematic,
> > > as they need to be flushed with BH being blocked.
> > >
> > > To cope with modern needs, TCP sockets have big sk_rcvbuf values,
> > > in the order of 16 MB.
> > > This means that backlog can hold thousands of packets, and things
> > > like TCP coalescing or collapsing on this amount of packets can
> > > lead to insane latency spikes, since BH are blocked for too long.
> > >
> > > It is time to make UDP/TCP stacks preemptible.
> > >
> > > Note that fast path still runs from BH handler.
> >
> > this looks pretty awesome.
>
> Yes, I am pretty excited ;)
>
> > the change will make the backlog run in bh enabled, so that one
> > large flow reciever will not penalize the rest of the system, right?
>
> Not only large flows, but flows with losses/reorders.
> Typically many flows are in this case when a congestion collapse
> happens.
>
>
>
> > but you're saying that prequeue is also expensive, but not touched
> > by this patchset? was it addressed by your eariler patch?
> > Or more work still tbd?
>
> prequeue is handled by "[2/6] tcp: do not block bh during prequeue
> processing"
got it. It applies to both v4 and v6, right?
> Note that I also sent a patch earlier (("tcp: give prequeue mode some
> care")) to control max size of prequeue to 32 packets.
>
> > I'm just trying to understand more about tcp stack.
> >
> Sure ;)
>
> I also have a patch to add scheduling point in sendmsg() (ie : draining
> the backlog if not empty) for each new skb added to the write queue.
>
> Since each skb is about 64KB (with GSO/TSO), it means an application no
> longer will hold the socket lock too long, even when doing a
> write()/sendmsg() of say 8 MB at once ;)
that would be awesome. I think map/reduce type jobs typically
do large sendmsg, so it should help p99 latency.
^ 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