* Re: [PATCH V5 net-next 2/3] net: huawei_cdc_ncm: Introduce the huawei_cdc_ncm driver
From: Oliver Neukum @ 2013-11-04 9:17 UTC (permalink / raw)
To: Bjørn Mork
Cc: Enrico Mioso, Greg Kroah-Hartman, David S. Miller,
Steve Glendinning, Robert de Vries, Hayes Wang, Freddy Xin,
Liu Junliang, open list, open list:USB NETWORKING DR...,
open list:NETWORKING DRIVERS, ModemManager-devel
In-Reply-To: <87fvrgwe0e.fsf@nemi.mork.no>
On Fri, 2013-11-01 at 12:35 +0100, Bjørn Mork wrote:
> So I believe we should do the update unconditionally, and but skip
> usb_autopm_put_interface if the get failed. Accordingly, these
> functions should always return 0 (not that there is anything currently
> checking the return anyway).
>
> I'll prepare patches for cdc-wdm, qmi_wwan and cdc_mbim.
Good plan.
Regards
Oliver
^ permalink raw reply
* Re: [PATCH net-next 17/24] net: cdc_ncm: use netif_* and dev_* instead of pr_*
From: Bjørn Mork @ 2013-11-04 8:59 UTC (permalink / raw)
To: Joe Perches; +Cc: netdev, linux-usb, Alexey Orishko
In-Reply-To: <1383302214.13084.6.camel@joe-AO722>
Joe Perches <joe@perches.com> writes:
> You did most all the multi-line statement
> alignment perfectly but missed a couple.
Damn! OK, that will teach me to do "checkpatch --strict" before
submitting.
> Maybe in a follow-on patch.
Yes, I'll fix this when I get around to the next series for this driver.
Thanks.
Bjørn
>> diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
> []
>> @@ -1031,17 +1035,13 @@ cdc_ncm_speed_change(struct usbnet *dev,
>> * device speed. Do print it instead.
>> */
>> if ((tx_speed > 1000000) && (rx_speed > 1000000)) {
>> - printk(KERN_INFO KBUILD_MODNAME
>> - ": %s: %u mbit/s downlink "
>> - "%u mbit/s uplink\n",
>> - dev->net->name,
>> + netif_info(dev, link, dev->net,
>> + "%u mbit/s downlink %u mbit/s uplink\n",
>> (unsigned int)(rx_speed / 1000000U),
>> (unsigned int)(tx_speed / 1000000U));
>> } else {
>> - printk(KERN_INFO KBUILD_MODNAME
>> - ": %s: %u kbit/s downlink "
>> - "%u kbit/s uplink\n",
>> - dev->net->name,
>> + netif_info(dev, link, dev->net,
>> + "%u kbit/s downlink %u kbit/s uplink\n",
>> (unsigned int)(rx_speed / 1000U),
>> (unsigned int)(tx_speed / 1000U));
>> }
^ permalink raw reply
* [PATCH net-next v6 3/3] net: cdc_ncm: remove non-standard NCM device IDs
From: Bjørn Mork @ 2013-11-04 8:50 UTC (permalink / raw)
To: netdev; +Cc: linux-usb, Enrico Mioso, Oliver Neukum, Bjørn Mork
In-Reply-To: <1383555049-24834-1-git-send-email-bjorn@mork.no>
From: Enrico Mioso <mrkiko.rs@gmail.com>
Remove device IDs of NCM-like (but not NCM-conformant) devices, that are
handled by the huawwei_cdc_ncm driver now.
Signed-off-by: Enrico Mioso <mrkiko.rs@gmail.com>
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
drivers/net/usb/cdc_ncm.c | 11 -----------
1 file changed, 11 deletions(-)
diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 1763195..f74786a 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -1186,17 +1186,6 @@ static const struct usb_device_id cdc_devs[] = {
.driver_info = (unsigned long)&wwan_info,
},
- /* Huawei NCM devices disguised as vendor specific */
- { USB_VENDOR_AND_INTERFACE_INFO(0x12d1, 0xff, 0x02, 0x16),
- .driver_info = (unsigned long)&wwan_info,
- },
- { USB_VENDOR_AND_INTERFACE_INFO(0x12d1, 0xff, 0x02, 0x46),
- .driver_info = (unsigned long)&wwan_info,
- },
- { USB_VENDOR_AND_INTERFACE_INFO(0x12d1, 0xff, 0x02, 0x76),
- .driver_info = (unsigned long)&wwan_info,
- },
-
/* Infineon(now Intel) HSPA Modem platform */
{ USB_DEVICE_AND_INTERFACE_INFO(0x1519, 0x0443,
USB_CLASS_COMM,
--
1.7.10.4
^ permalink raw reply related
* [PATCH net-next v6 1/3] net: cdc_ncm: Export cdc_ncm_{tx,rx}_fixup functions for re-use
From: Bjørn Mork @ 2013-11-04 8:50 UTC (permalink / raw)
To: netdev; +Cc: linux-usb, Enrico Mioso, Oliver Neukum, Bjørn Mork
In-Reply-To: <1383555049-24834-1-git-send-email-bjorn@mork.no>
From: Enrico Mioso <mrkiko.rs@gmail.com>
Some drivers implementing NCM-like protocols, may re-use those functions, as is
the case in the huawei_cdc_ncm driver.
Export them via EXPORT_SYMBOL_GPL, in accordance with how other functions have
been exported.
Signed-off-by: Enrico Mioso <mrkiko.rs@gmail.com>
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
drivers/net/usb/cdc_ncm.c | 6 ++++--
include/linux/usb/cdc_ncm.h | 3 +++
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 11c7033..1763195 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -830,7 +830,7 @@ static void cdc_ncm_txpath_bh(unsigned long param)
}
}
-static struct sk_buff *
+struct sk_buff *
cdc_ncm_tx_fixup(struct usbnet *dev, struct sk_buff *skb, gfp_t flags)
{
struct sk_buff *skb_out;
@@ -857,6 +857,7 @@ error:
return NULL;
}
+EXPORT_SYMBOL_GPL(cdc_ncm_tx_fixup);
/* verify NTB header and return offset of first NDP, or negative error */
int cdc_ncm_rx_verify_nth16(struct cdc_ncm_ctx *ctx, struct sk_buff *skb_in)
@@ -943,7 +944,7 @@ error:
}
EXPORT_SYMBOL_GPL(cdc_ncm_rx_verify_ndp16);
-static int cdc_ncm_rx_fixup(struct usbnet *dev, struct sk_buff *skb_in)
+int cdc_ncm_rx_fixup(struct usbnet *dev, struct sk_buff *skb_in)
{
struct sk_buff *skb;
struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
@@ -1019,6 +1020,7 @@ err_ndp:
error:
return 0;
}
+EXPORT_SYMBOL_GPL(cdc_ncm_rx_fixup);
static void
cdc_ncm_speed_change(struct usbnet *dev,
diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h
index 2300f74..c3fa807 100644
--- a/include/linux/usb/cdc_ncm.h
+++ b/include/linux/usb/cdc_ncm.h
@@ -125,5 +125,8 @@ void cdc_ncm_unbind(struct usbnet *dev, struct usb_interface *intf);
struct sk_buff *cdc_ncm_fill_tx_frame(struct usbnet *dev, struct sk_buff *skb, __le32 sign);
int cdc_ncm_rx_verify_nth16(struct cdc_ncm_ctx *ctx, struct sk_buff *skb_in);
int cdc_ncm_rx_verify_ndp16(struct sk_buff *skb_in, int ndpoffset);
+struct sk_buff *
+cdc_ncm_tx_fixup(struct usbnet *dev, struct sk_buff *skb, gfp_t flags);
+int cdc_ncm_rx_fixup(struct usbnet *dev, struct sk_buff *skb_in);
#endif /* __LINUX_USB_CDC_NCM_H */
--
1.7.10.4
^ permalink raw reply related
* [PATCH net-next v6 2/3] net: huawei_cdc_ncm: Introduce the huawei_cdc_ncm driver
From: Bjørn Mork @ 2013-11-04 8:50 UTC (permalink / raw)
To: netdev; +Cc: linux-usb, Enrico Mioso, Oliver Neukum, Bjørn Mork
In-Reply-To: <1383555049-24834-1-git-send-email-bjorn@mork.no>
From: Enrico Mioso <mrkiko.rs@gmail.com>
This driver supports devices using the NCM protocol as an encapsulation layer
for other protocols, like the E3131 Huawei 3G modem. This drivers approach was
heavily inspired by the qmi_wwan/cdc_mbim approach & code model.
Signed-off-by: Enrico Mioso <mrkiko.rs@gmail.com>
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
drivers/net/usb/Kconfig | 15 +++
drivers/net/usb/Makefile | 1 +
drivers/net/usb/huawei_cdc_ncm.c | 230 ++++++++++++++++++++++++++++++++++++++
3 files changed, 246 insertions(+)
create mode 100644 drivers/net/usb/huawei_cdc_ncm.c
diff --git a/drivers/net/usb/Kconfig b/drivers/net/usb/Kconfig
index 40db312..85e4a01 100644
--- a/drivers/net/usb/Kconfig
+++ b/drivers/net/usb/Kconfig
@@ -242,6 +242,21 @@ config USB_NET_CDC_NCM
* ST-Ericsson M343 HSPA Mobile Broadband Modem (reference design)
* Ericsson F5521gw Mobile Broadband Module
+config USB_NET_HUAWEI_CDC_NCM
+ tristate "Huawei NCM embedded AT channel support"
+ depends on USB_USBNET
+ select USB_WDM
+ select USB_NET_CDC_NCM
+ help
+ This driver supports huawei-style NCM devices, that use NCM as a
+ transport for other protocols, usually an embedded AT channel.
+ Good examples are:
+ * Huawei E3131
+ * Huawei E3251
+
+ To compile this driver as a module, choose M here: the module will be
+ called huawei_cdc_ncm.ko.
+
config USB_NET_CDC_MBIM
tristate "CDC MBIM support"
depends on USB_USBNET
diff --git a/drivers/net/usb/Makefile b/drivers/net/usb/Makefile
index 8b342cf..b17b5e8 100644
--- a/drivers/net/usb/Makefile
+++ b/drivers/net/usb/Makefile
@@ -32,6 +32,7 @@ obj-$(CONFIG_USB_IPHETH) += ipheth.o
obj-$(CONFIG_USB_SIERRA_NET) += sierra_net.o
obj-$(CONFIG_USB_NET_CX82310_ETH) += cx82310_eth.o
obj-$(CONFIG_USB_NET_CDC_NCM) += cdc_ncm.o
+obj-$(CONFIG_USB_NET_HUAWEI_CDC_NCM) += huawei_cdc_ncm.o
obj-$(CONFIG_USB_VL600) += lg-vl600.o
obj-$(CONFIG_USB_NET_QMI_WWAN) += qmi_wwan.o
obj-$(CONFIG_USB_NET_CDC_MBIM) += cdc_mbim.o
diff --git a/drivers/net/usb/huawei_cdc_ncm.c b/drivers/net/usb/huawei_cdc_ncm.c
new file mode 100644
index 0000000..312178d
--- /dev/null
+++ b/drivers/net/usb/huawei_cdc_ncm.c
@@ -0,0 +1,230 @@
+/* huawei_cdc_ncm.c - handles Huawei devices using the CDC NCM protocol as
+ * transport layer.
+ * Copyright (C) 2013 Enrico Mioso <mrkiko.rs@gmail.com>
+ *
+ *
+ * ABSTRACT:
+ * This driver handles devices resembling the CDC NCM standard, but
+ * encapsulating another protocol inside it. An example are some Huawei 3G
+ * devices, exposing an embedded AT channel where you can set up the NCM
+ * connection.
+ * This code has been heavily inspired by the cdc_mbim.c driver, which is
+ * Copyright (c) 2012 Smith Micro Software, Inc.
+ * Copyright (c) 2012 Bjørn Mork <bjorn@mork.no>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ */
+
+#include <linux/module.h>
+#include <linux/netdevice.h>
+#include <linux/ethtool.h>
+#include <linux/if_vlan.h>
+#include <linux/ip.h>
+#include <linux/mii.h>
+#include <linux/usb.h>
+#include <linux/usb/cdc.h>
+#include <linux/usb/usbnet.h>
+#include <linux/usb/cdc-wdm.h>
+#include <linux/usb/cdc_ncm.h>
+
+/* Driver data */
+struct huawei_cdc_ncm_state {
+ struct cdc_ncm_ctx *ctx;
+ atomic_t pmcount;
+ struct usb_driver *subdriver;
+ struct usb_interface *control;
+ struct usb_interface *data;
+};
+
+static int huawei_cdc_ncm_manage_power(struct usbnet *usbnet_dev, int on)
+{
+ struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
+ int rv;
+
+ if ((on && atomic_add_return(1, &drvstate->pmcount) == 1) ||
+ (!on && atomic_dec_and_test(&drvstate->pmcount))) {
+ rv = usb_autopm_get_interface(usbnet_dev->intf);
+ usbnet_dev->intf->needs_remote_wakeup = on;
+ if (!rv)
+ usb_autopm_put_interface(usbnet_dev->intf);
+ }
+ return 0;
+}
+
+static int huawei_cdc_ncm_wdm_manage_power(struct usb_interface *intf,
+ int status)
+{
+ struct usbnet *usbnet_dev = usb_get_intfdata(intf);
+
+ /* can be called while disconnecting */
+ if (!usbnet_dev)
+ return 0;
+
+ return huawei_cdc_ncm_manage_power(usbnet_dev, status);
+}
+
+
+static int huawei_cdc_ncm_bind(struct usbnet *usbnet_dev,
+ struct usb_interface *intf)
+{
+ struct cdc_ncm_ctx *ctx;
+ struct usb_driver *subdriver = ERR_PTR(-ENODEV);
+ int ret = -ENODEV;
+ struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
+
+ /* altsetting should always be 1 for NCM devices - so we hard-coded
+ * it here
+ */
+ ret = cdc_ncm_bind_common(usbnet_dev, intf, 1);
+ if (ret)
+ goto err;
+
+ ctx = drvstate->ctx;
+
+ if (usbnet_dev->status)
+ /* CDC-WMC r1.1 requires wMaxCommand to be "at least 256
+ * decimal (0x100)"
+ */
+ subdriver = usb_cdc_wdm_register(ctx->control,
+ &usbnet_dev->status->desc,
+ 256, /* wMaxCommand */
+ huawei_cdc_ncm_wdm_manage_power);
+ if (IS_ERR(subdriver)) {
+ ret = PTR_ERR(subdriver);
+ cdc_ncm_unbind(usbnet_dev, intf);
+ goto err;
+ }
+
+ /* Prevent usbnet from using the status descriptor */
+ usbnet_dev->status = NULL;
+
+ drvstate->subdriver = subdriver;
+
+err:
+ return ret;
+}
+
+static void huawei_cdc_ncm_unbind(struct usbnet *usbnet_dev,
+ struct usb_interface *intf)
+{
+ struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
+ struct cdc_ncm_ctx *ctx = drvstate->ctx;
+
+ if (drvstate->subdriver && drvstate->subdriver->disconnect)
+ drvstate->subdriver->disconnect(ctx->control);
+ drvstate->subdriver = NULL;
+
+ cdc_ncm_unbind(usbnet_dev, intf);
+}
+
+static int huawei_cdc_ncm_suspend(struct usb_interface *intf,
+ pm_message_t message)
+{
+ int ret = 0;
+ struct usbnet *usbnet_dev = usb_get_intfdata(intf);
+ struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
+ struct cdc_ncm_ctx *ctx = drvstate->ctx;
+
+ if (ctx == NULL) {
+ ret = -ENODEV;
+ goto error;
+ }
+
+ ret = usbnet_suspend(intf, message);
+ if (ret < 0)
+ goto error;
+
+ if (intf == ctx->control &&
+ drvstate->subdriver &&
+ drvstate->subdriver->suspend)
+ ret = drvstate->subdriver->suspend(intf, message);
+ if (ret < 0)
+ usbnet_resume(intf);
+
+error:
+ return ret;
+}
+
+static int huawei_cdc_ncm_resume(struct usb_interface *intf)
+{
+ int ret = 0;
+ struct usbnet *usbnet_dev = usb_get_intfdata(intf);
+ struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
+ bool callsub;
+ struct cdc_ncm_ctx *ctx = drvstate->ctx;
+
+ /* should we call subdriver's resume function? */
+ callsub =
+ (intf == ctx->control &&
+ drvstate->subdriver &&
+ drvstate->subdriver->resume);
+
+ if (callsub)
+ ret = drvstate->subdriver->resume(intf);
+ if (ret < 0)
+ goto err;
+ ret = usbnet_resume(intf);
+ if (ret < 0 && callsub)
+ drvstate->subdriver->suspend(intf, PMSG_SUSPEND);
+err:
+ return ret;
+}
+
+static int huawei_cdc_ncm_check_connect(struct usbnet *usbnet_dev)
+{
+ struct cdc_ncm_ctx *ctx;
+
+ ctx = (struct cdc_ncm_ctx *)usbnet_dev->data[0];
+
+ if (ctx == NULL)
+ return 1; /* disconnected */
+
+ return !ctx->connected;
+}
+
+static const struct driver_info huawei_cdc_ncm_info = {
+ .description = "Huawei CDC NCM device",
+ .flags = FLAG_NO_SETINT | FLAG_MULTI_PACKET | FLAG_WWAN,
+ .bind = huawei_cdc_ncm_bind,
+ .unbind = huawei_cdc_ncm_unbind,
+ .check_connect = huawei_cdc_ncm_check_connect,
+ .manage_power = huawei_cdc_ncm_manage_power,
+ .rx_fixup = cdc_ncm_rx_fixup,
+ .tx_fixup = cdc_ncm_tx_fixup,
+};
+
+static const struct usb_device_id huawei_cdc_ncm_devs[] = {
+ /* Huawei NCM devices disguised as vendor specific */
+ { USB_VENDOR_AND_INTERFACE_INFO(0x12d1, 0xff, 0x02, 0x16),
+ .driver_info = (unsigned long)&huawei_cdc_ncm_info,
+ },
+ { USB_VENDOR_AND_INTERFACE_INFO(0x12d1, 0xff, 0x02, 0x46),
+ .driver_info = (unsigned long)&huawei_cdc_ncm_info,
+ },
+ { USB_VENDOR_AND_INTERFACE_INFO(0x12d1, 0xff, 0x02, 0x76),
+ .driver_info = (unsigned long)&huawei_cdc_ncm_info,
+ },
+
+ /* Terminating entry */
+ {
+ },
+};
+MODULE_DEVICE_TABLE(usb, huawei_cdc_ncm_devs);
+
+static struct usb_driver huawei_cdc_ncm_driver = {
+ .name = "huawei_cdc_ncm",
+ .id_table = huawei_cdc_ncm_devs,
+ .probe = usbnet_probe,
+ .disconnect = usbnet_disconnect,
+ .suspend = huawei_cdc_ncm_suspend,
+ .resume = huawei_cdc_ncm_resume,
+ .reset_resume = huawei_cdc_ncm_resume,
+ .supports_autosuspend = 1,
+ .disable_hub_initiated_lpm = 1,
+};
+module_usb_driver(huawei_cdc_ncm_driver);
+MODULE_AUTHOR("Enrico Mioso <mrkiko.rs@gmail.com>");
+MODULE_DESCRIPTION("USB CDC NCM host driver with encapsulated protocol support");
+MODULE_LICENSE("GPL");
--
1.7.10.4
^ permalink raw reply related
* [PATCH net-next v6 0/3] The huawei_cdc_ncm driver
From: Bjørn Mork @ 2013-11-04 8:50 UTC (permalink / raw)
To: netdev-u79uwXL29TY76Z2rM5mHXA
Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA, Enrico Mioso, Oliver Neukum,
Bjørn Mork
Enrico has been kind enough to let me repost his driver with the changes
requested by Oliver Neukum during the last review of this series.
The changes I have made from Enricos original v5 series to this version
are:
v6:
- fix to avoid corrupting drvstate->pmcount
- fix error return value from huawei_cdc_ncm_suspend()
- drop redundant testing for subdriver->suspend during resume
- broke a few lines to keep within the 80 columns recommendation
- rebased on top of current net-next
Enrico's orginal introduction to the v5 series follows below. It explains
the background much better than I can.
Bjørn
[quote Enrico Mioso]
So this is a new, revised, edition of the huawei_cdc_ncm.c driver, which
supports devices resembling the NCM standard, but using it also as a mean
to encapsulate other protocols, as is the case for the Huawei E3131 and
E3251 modem devices.
Some precisations are needed however - and I encourage discussion on this: and
that's why I'm sending this message with a broader CC.
Merging those patches might change:
- the way Modem Manager interacts with those devices
- some regressions might be possible if there are some unknown firmware
variants around (Franko?)
First of all: I observed the behaviours of two devices.
Huawei E3131: this device doesn't accept NDIS setup requests unless they're
sent via the embedded AT channel exposed by this driver.
So actually we gain funcionality in this case!
The second case, is the Huawei E3251: which works with standard NCM driver,
still exposing an AT embedded channel. Whith this patch set applied, you gain
some funcionality, loosing the ability to catch standard NCM events for now.
The device will work in both ways with no problems, but this has to be
acknowledged and discussed. Might be we can develop this driver further to
change this, when more devices are tested.
We where thinking Huawei changed their interfaces on new devices - but probably
this driver only works around a nice firmware bug present in E3131, which
prevented the modem from being used in NDIS mode.
I think committing this is definitely wortth-while, since it will allow for
more Huawei devices to be used without serial connection. Some devices like the
E3251 also, reports some status information only via the embedded AT channel,
at least in my case.
Note: I'm not subscribed to any list except the Modem Manager's one, so please
CC me, thanks!!
[/quote]
Enrico Mioso (3):
net: cdc_ncm: Export cdc_ncm_{tx,rx}_fixup functions for re-use
net: huawei_cdc_ncm: Introduce the huawei_cdc_ncm driver
net: cdc_ncm: remove non-standard NCM device IDs
drivers/net/usb/Kconfig | 15 +++
drivers/net/usb/Makefile | 1 +
drivers/net/usb/cdc_ncm.c | 17 +--
drivers/net/usb/huawei_cdc_ncm.c | 230 ++++++++++++++++++++++++++++++++++++++
include/linux/usb/cdc_ncm.h | 3 +
5 files changed, 253 insertions(+), 13 deletions(-)
create mode 100644 drivers/net/usb/huawei_cdc_ncm.c
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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: [PATCH v3 net-next] net: introduce dev_set_forwarding()
From: Herbert Xu @ 2013-11-04 7:21 UTC (permalink / raw)
To: Eric Dumazet
Cc: Ben Hutchings, David Miller, christoph.paasch, netdev, hkchu,
mwdalton
In-Reply-To: <1383549041.4291.110.camel@edumazet-glaptop2.roam.corp.google.com>
On Sun, Nov 03, 2013 at 11:10:41PM -0800, Eric Dumazet wrote:
>
> Using frag_list for skb meant to be delivered to local stack is mostly
> fine. Its for forwarding that its not a win, since no driver actually
> supports frag_list and we revert to skb_segment().
Sigh.
Most of the gain these days isn't coming from the hardware anymore,
especially for virtualisation where the network stack is at least
twice as long. The gain is in paying the cost of the network once
instead of n times for an aggregation of n packets. So with your
mega-GRO patch, the choice comes down to paying for one trip or
three trips. The difference may not be trivial, even for a router.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH v3 net-next] net: introduce dev_set_forwarding()
From: Eric Dumazet @ 2013-11-04 7:10 UTC (permalink / raw)
To: Herbert Xu
Cc: Ben Hutchings, David Miller, christoph.paasch, netdev, hkchu,
mwdalton
In-Reply-To: <20131104062650.GA23755@gondor.apana.org.au>
On Mon, 2013-11-04 at 14:26 +0800, Herbert Xu wrote:
> Also if your scenario had all GRO candidates then not using
> frag_list would seem to be a bad workaround for an underlying
> latency problem. Rather than arbitrarily limiting the aggregation
> to 22K surely it would make more sense to limit it based on your
> actual latency requirements?
I have never limited GRO aggregation to 22K.
You did this in commit 81705ad1b2f926d
("gro: Do not merge paged packets into frag_list")
I exactly implemented what you suggested in your patch :
"In future we can optimise this further by doing frag_list merging
but making sure that we continue to fill in the page array."
Using frag_list for skb meant to be delivered to local stack is mostly
fine. Its for forwarding that its not a win, since no driver actually
supports frag_list and we revert to skb_segment().
So my latest patch about _not_ building fat skbs with frag_list on a
router makes sense.
^ permalink raw reply
* Re: [PATCH v3 net-next] net: introduce dev_set_forwarding()
From: Herbert Xu @ 2013-11-04 7:03 UTC (permalink / raw)
To: Eric Dumazet
Cc: Ben Hutchings, David Miller, christoph.paasch, netdev, hkchu,
mwdalton
In-Reply-To: <1383547581.4291.91.camel@edumazet-glaptop2.roam.corp.google.com>
On Sun, Nov 03, 2013 at 10:46:21PM -0800, Eric Dumazet wrote:
> On Mon, 2013-11-04 at 14:22 +0800, Herbert Xu wrote:
>
> > So what NICs were you using that had this issue?
>
> Its a generic issue, really.
I'd still like answer because we also tested large numbers of
flows and have not seen this.
Thanks,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH v3 net-next] net: introduce dev_set_forwarding()
From: Eric Dumazet @ 2013-11-04 6:46 UTC (permalink / raw)
To: Herbert Xu
Cc: Ben Hutchings, David Miller, christoph.paasch, netdev, hkchu,
mwdalton
In-Reply-To: <20131104062202.GA23719@gondor.apana.org.au>
On Mon, 2013-11-04 at 14:22 +0800, Herbert Xu wrote:
> So what NICs were you using that had this issue?
Its a generic issue, really.
It depends on how many flows are mixed per RX queues.
^ permalink raw reply
* Re: [PATCH v3 net-next] net: introduce dev_set_forwarding()
From: Herbert Xu @ 2013-11-04 6:26 UTC (permalink / raw)
To: Eric Dumazet
Cc: Ben Hutchings, David Miller, christoph.paasch, netdev, hkchu,
mwdalton
In-Reply-To: <20131104062202.GA23719@gondor.apana.org.au>
On Mon, Nov 04, 2013 at 02:22:02PM +0800, Herbert Xu wrote:
>
> > We had these latencies only dealing with TCP packets, all GRO
> > candidates.
> >
> > Really, I think we have used GRO at large scale here at Google ;)
>
> So what NICs were you using that had this issue?
Also if your scenario had all GRO candidates then not using
frag_list would seem to be a bad workaround for an underlying
latency problem. Rather than arbitrarily limiting the aggregation
to 22K surely it would make more sense to limit it based on your
actual latency requirements?
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH v3 net-next] net: introduce dev_set_forwarding()
From: Herbert Xu @ 2013-11-04 6:22 UTC (permalink / raw)
To: Eric Dumazet
Cc: Ben Hutchings, David Miller, christoph.paasch, netdev, hkchu,
mwdalton
In-Reply-To: <1383545155.4291.89.camel@edumazet-glaptop2.roam.corp.google.com>
On Sun, Nov 03, 2013 at 10:05:55PM -0800, Eric Dumazet wrote:
>
> > I think we can definitely improve our behaviour the CPU-bound case.
> > Right now if we encounter something we can't hold for GRO we
> > start processing it right away. Instead we can place it in a
> > list for later processing together with the GRO packets.
> >
> > This way GRO packets are not penalised by non-GRO packets.
> >
> > You can then use the usual NAPI budget to minimise latency and
> > ensure scheduling fairness.
>
> We had these latencies only dealing with TCP packets, all GRO
> candidates.
>
> Really, I think we have used GRO at large scale here at Google ;)
So what NICs were you using that had this issue?
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH v3 net-next] net: introduce dev_set_forwarding()
From: Eric Dumazet @ 2013-11-04 6:05 UTC (permalink / raw)
To: Herbert Xu
Cc: Ben Hutchings, David Miller, christoph.paasch, netdev, hkchu,
mwdalton
In-Reply-To: <20131104052321.GA23252@gondor.apana.org.au>
On Mon, 2013-11-04 at 13:23 +0800, Herbert Xu wrote:
> On Sun, Nov 03, 2013 at 09:00:54PM -0800, Eric Dumazet wrote:
> > On Mon, 2013-11-04 at 12:29 +0800, Herbert Xu wrote:
> >
> > > Have you actually measured this? The latency added by GRO is pure
> > > processing overhead. This is tiny when compared to the time NAPI takes
> > > to wait.
> >
> > Please take a look at
> >
> > 2e71a6f8084e net: gro: selective flush of packets
>
> This is a different problem altogether. I was worried about the
> latency in cases where we're idle and waiting for new data, while
> you're worried about the latency in the CPU-bound case.
Idle case, you very rarely cant keep up building skbs with 16 MSS.
>
> I think we can definitely improve our behaviour the CPU-bound case.
> Right now if we encounter something we can't hold for GRO we
> start processing it right away. Instead we can place it in a
> list for later processing together with the GRO packets.
>
> This way GRO packets are not penalised by non-GRO packets.
>
> You can then use the usual NAPI budget to minimise latency and
> ensure scheduling fairness.
We had these latencies only dealing with TCP packets, all GRO
candidates.
Really, I think we have used GRO at large scale here at Google ;)
^ permalink raw reply
* [GIT] Networking
From: David Miller @ 2013-11-04 6:00 UTC (permalink / raw)
To: torvalds; +Cc: akpm, netdev, linux-kernel
I'm sending a pull request of these lingering bug fixes for networking
before the normal merge window material because some of this stuff I'd
like to get to -stable ASAP.
1) cxgb3 stopped working on 32-bit machines, fix from Ben Hutchings.
2) Structures passed via netlink for netfilter logging are not fully
initialized. From Mathias Krause.
3) Properly unlink upper openvswitch device during notifications,
from Alexei Starovoitov.
4) Fix race conditions involving access to the IP compression scratch
buffer, from Michal Kubrecek.
5) We don't handle the expiration of MTU information contained in
ipv6 routes sometimes, fix from Hannes Frederic Sowa.
6) With Fast Open we can miscompute the TCP SYN/ACK RTT, from Yuchung
Cheng.
7) Don't take TCP RTT sample when an ACK doesn't acknowledge new data,
also from Yuchung Cheng.
8) The decreased IPSEC garbage collection threshold causes problems
for some people, bump it back up. From Steffen Klassert.
9) Fix skb->truesize calculated by tcp_tso_segment(), from Eric
Dumazet.
10) flow_dissector doesn't validate packet lengths sufficiently, from
Jason Wang.
Please pull, thanks a lot!
The following changes since commit 320437af954cbe66478f1f5e8b34cb5a8d072191:
Merge branch 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/s390/linux (2013-10-23 08:10:25 +0100)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/davem/net master
for you to fetch changes up to c32b7dfbb1dfb3f0a68f250deff65103c8bb704a:
net/mlx4_core: Fix call to __mlx4_unregister_mac (2013-11-04 00:51:10 -0500)
----------------------------------------------------------------
Alexei Starovoitov (1):
openvswitch: fix vport-netdev unregister
Alistair Popple (2):
ibm emac: Don't call napi_complete if napi_reschedule failed
ibm emac: Fix locking for enable/disable eob irq
Antonio Quartulli (1):
netpoll: fix rx_hook() interface by passing the skb
Ariel Elior (1):
bnx2x: Disable VF access on PF removal
Ben Hutchings (1):
cxgb3: Fix length calculation in write_ofld_wr() on 32-bit architectures
Daniel Borkmann (2):
net: sctp: fix ASCONF to allow non SCTP_ADDR_SRC addresses in ipv6
net: sctp: do not trigger BUG_ON in sctp_cmd_delete_tcb
David S. Miller (6):
Merge branch 'master' of git://git.kernel.org/.../pablo/nf
Merge branch 'qlcnic'
Merge branch 'master' of git://git.kernel.org/.../pablo/nf
Merge branch 'fixes' of git://git.kernel.org/.../jesse/openvswitch
Merge branch 'master' of git://git.kernel.org/.../klassert/ipsec
Merge branch 'fixes-for-3.12' of git://gitorious.org/linux-can/linux-can
Dmitry Kravkov (1):
bnx2x: prevent FW assert on low mem during unload
Eric Dumazet (2):
pkt_sched: fq: clear time_next_packet for reused flows
tcp: gso: fix truesize tracking
Freddy Xin (1):
ax88179_178a: Remove AX_MEDIUM_ALWAYS_ONE bit in AX_MEDIUM_STATUS_MODE register to avoid TX throttling
Hannes Frederic Sowa (2):
ipv6: reset dst.expires value when clearing expire flag
ipv6: ip6_dst_check needs to check for expired dst_entries
Holger Eitzenberger (1):
netfilter: xt_NFQUEUE: fix --queue-bypass regression
Jack Morgenstein (1):
net/mlx4_core: Fix call to __mlx4_unregister_mac
Jason Wang (2):
virtio-net: correctly handle cpu hotplug notifier during resuming
net: flow_dissector: fail on evil iph->ihl
Markus Pargmann (1):
can: c_can: Fix RX message handling, handle lost message before EOB
Masanari Iida (1):
doc:net: Fix typo in Documentation/networking
Mathias Krause (2):
netfilter: ebt_ulog: fix info leaks
netfilter: ipt_ULOG: fix info leaks
Michael Drüing (1):
net: x25: Fix dead URLs in Kconfig
Michal Kubecek (1):
xfrm: prevent ipcomp scratch buffer race condition
Nathan Hintz (1):
bgmac: don't update slot on skb alloc/dma mapping error
Nikolay Aleksandrov (2):
netconsole: fix NULL pointer dereference
netconsole: fix multiple race conditions
Olivier Sobrie (1):
can: kvaser_usb: fix usb endpoints detection
Randy Dunlap (1):
Documentation/networking: netdev-FAQ typo corrections
Sebastian Siewior (1):
net: wan: sbni: remove assembly crc32 code
Shahed Shaikh (2):
qlcnic: Do not force adapter to perform LRO without destination IP check
qlcnic: Do not read QLCNIC_FW_CAPABILITY_MORE_CAPS bit for 83xx adapter
Somnath Kotur (1):
be2net: Warn users of possible broken functionality on BE2 cards with very old FW versions with latest driver
Steffen Klassert (2):
xfrm: Increase the garbage collector threshold
xfrm: Fix null pointer dereference when decoding sessions
Vlad Yasevich (1):
bridge: pass correct vlan id to multicast code
Wei Liu (1):
xen-netback: use jiffies_64 value to calculate credit timeout
Will Deacon (1):
netfilter: x_tables: fix ordering of jumpstack allocation and table update
Yuchung Cheng (3):
tcp: fix SYNACK RTT estimation in Fast Open
tcp: only take RTT from timestamps if new data is acked
tcp: do not rearm RTO when future data are sacked
Documentation/networking/dccp.txt | 4 +--
Documentation/networking/e100.txt | 2 +-
Documentation/networking/ieee802154.txt | 4 +--
Documentation/networking/l2tp.txt | 2 +-
Documentation/networking/netdev-FAQ.txt | 24 ++++++++--------
Documentation/networking/netlink_mmap.txt | 6 ++--
Documentation/networking/operstates.txt | 4 +--
Documentation/networking/rxrpc.txt | 2 +-
Documentation/networking/stmmac.txt | 8 +++---
Documentation/networking/vortex.txt | 4 +--
Documentation/networking/x25-iface.txt | 2 +-
drivers/net/can/c_can/c_can.c | 6 ++--
drivers/net/can/usb/kvaser_usb.c | 20 +++++++++-----
drivers/net/ethernet/broadcom/bgmac.c | 20 ++++++++++----
drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 10 +++----
drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c | 16 ++++++++++-
drivers/net/ethernet/chelsio/cxgb3/sge.c | 3 +-
drivers/net/ethernet/emulex/benet/be.h | 9 ++++++
drivers/net/ethernet/emulex/benet/be_main.c | 6 ++++
drivers/net/ethernet/ibm/emac/mal.c | 16 +++++------
drivers/net/ethernet/mellanox/mlx4/cmd.c | 2 +-
drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_hw.c | 6 ++--
drivers/net/ethernet/qlogic/qlcnic/qlcnic_hw.c | 7 ++---
drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c | 6 ++--
drivers/net/netconsole.c | 18 +++++++-----
drivers/net/usb/ax88179_178a.c | 11 ++++----
drivers/net/virtio_net.c | 13 ++++-----
drivers/net/wan/sbni.c | 89 ------------------------------------------------------------
drivers/net/xen-netback/common.h | 1 +
drivers/net/xen-netback/interface.c | 3 +-
drivers/net/xen-netback/netback.c | 10 +++----
include/linux/netpoll.h | 5 ++--
include/net/ip6_fib.h | 1 +
net/bridge/br_device.c | 2 +-
net/bridge/br_input.c | 2 +-
net/bridge/br_multicast.c | 44 +++++++++++++-----------------
net/bridge/br_private.h | 6 ++--
net/bridge/netfilter/ebt_ulog.c | 9 ++----
net/core/flow_dissector.c | 2 +-
net/core/netpoll.c | 31 ++++++++++++---------
net/ipv4/netfilter/arp_tables.c | 5 ++++
net/ipv4/netfilter/ip_tables.c | 5 ++++
net/ipv4/netfilter/ipt_ULOG.c | 7 +----
net/ipv4/tcp_input.c | 34 +++++++++++++++++------
net/ipv4/tcp_offload.c | 13 ++++-----
net/ipv4/xfrm4_policy.c | 8 ++++--
net/ipv6/netfilter/ip6_tables.c | 5 ++++
net/ipv6/route.c | 9 ++++--
net/ipv6/xfrm6_policy.c | 8 ++++--
net/netfilter/x_tables.c | 7 ++++-
net/netfilter/xt_NFQUEUE.c | 7 ++++-
net/openvswitch/dp_notify.c | 7 +++--
net/openvswitch/vport-netdev.c | 16 +++++++++--
net/openvswitch/vport-netdev.h | 1 +
net/sched/sch_fq.c | 1 +
net/sctp/ipv6.c | 4 ++-
net/sctp/sm_sideeffect.c | 1 -
net/x25/Kconfig | 4 +--
net/xfrm/xfrm_ipcomp.c | 12 ++++----
59 files changed, 303 insertions(+), 287 deletions(-)
^ permalink raw reply
* Re: [PATCH net] net/mlx4_core: Fix call to __mlx4_unregister_mac
From: David Miller @ 2013-11-04 5:51 UTC (permalink / raw)
To: ogerlitz; +Cc: netdev, amirv, jackm
In-Reply-To: <1383465847-28719-1-git-send-email-ogerlitz@mellanox.com>
From: Or Gerlitz <ogerlitz@mellanox.com>
Date: Sun, 3 Nov 2013 10:04:07 +0200
> From: Jack Morgenstein <jackm@dev.mellanox.co.il>
>
> In function mlx4_master_deactivate_admin_state() __mlx4_unregister_mac was
> called using the MAC index. It should be called with the value of the MAC itself.
>
> Signed-off-by: Jack Morgenstein <jackm@dev.mellanox.co.il>
> Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: pull-request: can 2013-10-31
From: David Miller @ 2013-11-04 5:48 UTC (permalink / raw)
To: mkl; +Cc: netdev, linux-can, kernel
In-Reply-To: <1383247490-26984-1-git-send-email-mkl@pengutronix.de>
From: Marc Kleine-Budde <mkl@pengutronix.de>
Date: Thu, 31 Oct 2013 20:24:48 +0100
> The first patch fixes a problem in the c_can's RX message handling, which can
> lead to an endless interrupt loop under heavy load if messages are lost. The
> second patch is by Olivier Sobrie and fixes the endpoint detection of the
> kvaser_usb driver, which is needed for some devices.
Pulled, thanks Marc.
^ permalink raw reply
* Re: [PATCH net] net: sctp: do not trigger BUG_ON in sctp_cmd_delete_tcb
From: David Miller @ 2013-11-04 5:47 UTC (permalink / raw)
To: dborkman; +Cc: netdev, linux-sctp, vyasevich
In-Reply-To: <1383207212-21658-1-git-send-email-dborkman@redhat.com>
From: Daniel Borkmann <dborkman@redhat.com>
Date: Thu, 31 Oct 2013 09:13:32 +0100
> Introduced in f9e42b853523 ("net: sctp: sideeffect: throw BUG if
> primary_path is NULL"), we intended to find a buggy assoc that's
> part of the assoc hash table with a primary_path that is NULL.
> However, we better remove the BUG_ON for now and find a more
> suitable place to assert for these things as Mark reports that
> this also triggers the bug when duplication cookie processing
> happens, and the assoc is not part of the hash table (so all
> good in this case). Such a situation can for example easily be
> reproduced by:
>
> tc qdisc add dev eth0 root handle 1: prio bands 2 priomap 1 1 1 1 1 1
> tc qdisc add dev eth0 parent 1:2 handle 20: netem loss 20%
> tc filter add dev eth0 protocol ip parent 1: prio 2 u32 match ip \
> protocol 132 0xff match u8 0x0b 0xff at 32 flowid 1:2
>
> This drops 20% of COOKIE-ACK packets. After some follow-up
> discussion with Vlad we came to the conclusion that for now we
> should still better remove this BUG_ON() assertion, and come up
> with two follow-ups later on, that is, i) find a more suitable
> place for this assertion, and possibly ii) have a special
> allocator/initializer for such kind of temporary assocs.
>
> Reported-by: Mark Thomas <Mark.Thomas@metaswitch.com>
> Signed-off-by: Vlad Yasevich <vyasevich@gmail.com>
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH v3 net-next] net: introduce dev_set_forwarding()
From: Herbert Xu @ 2013-11-04 5:23 UTC (permalink / raw)
To: Eric Dumazet
Cc: Ben Hutchings, David Miller, christoph.paasch, netdev, hkchu,
mwdalton
In-Reply-To: <1383541254.4291.82.camel@edumazet-glaptop2.roam.corp.google.com>
On Sun, Nov 03, 2013 at 09:00:54PM -0800, Eric Dumazet wrote:
> On Mon, 2013-11-04 at 12:29 +0800, Herbert Xu wrote:
>
> > Have you actually measured this? The latency added by GRO is pure
> > processing overhead. This is tiny when compared to the time NAPI takes
> > to wait.
>
> Please take a look at
>
> 2e71a6f8084e net: gro: selective flush of packets
This is a different problem altogether. I was worried about the
latency in cases where we're idle and waiting for new data, while
you're worried about the latency in the CPU-bound case.
I think we can definitely improve our behaviour the CPU-bound case.
Right now if we encounter something we can't hold for GRO we
start processing it right away. Instead we can place it in a
list for later processing together with the GRO packets.
This way GRO packets are not penalised by non-GRO packets.
You can then use the usual NAPI budget to minimise latency and
ensure scheduling fairness.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH v3 net-next] net: introduce dev_set_forwarding()
From: Eric Dumazet @ 2013-11-04 5:00 UTC (permalink / raw)
To: Herbert Xu
Cc: Ben Hutchings, David Miller, christoph.paasch, netdev, hkchu,
mwdalton
In-Reply-To: <20131104042913.GA23021@gondor.apana.org.au>
On Mon, 2013-11-04 at 12:29 +0800, Herbert Xu wrote:
> On Sun, Nov 03, 2013 at 08:23:02PM -0800, Eric Dumazet wrote:
> >
> > Well, GRO adds latencies for sure.
> >
> > You seem to assume the transmit only can happen when NAPI is done, but
> > its not true. As soon as GRO fills one packet (reaches max capacity),
> > packet is delivered and forwarded, even if NAPI handler is not yet
> > complete for the flow.
>
> Have you actually measured this? The latency added by GRO is pure
> processing overhead. This is tiny when compared to the time NAPI takes
> to wait.
Please take a look at
2e71a6f8084e net: gro: selective flush of packets
^ permalink raw reply
* Re: [PATCH v3 net-next] net: introduce dev_set_forwarding()
From: Herbert Xu @ 2013-11-04 4:29 UTC (permalink / raw)
To: Eric Dumazet
Cc: Ben Hutchings, David Miller, christoph.paasch, netdev, hkchu,
mwdalton
In-Reply-To: <1383538982.4291.80.camel@edumazet-glaptop2.roam.corp.google.com>
On Sun, Nov 03, 2013 at 08:23:02PM -0800, Eric Dumazet wrote:
>
> Well, GRO adds latencies for sure.
>
> You seem to assume the transmit only can happen when NAPI is done, but
> its not true. As soon as GRO fills one packet (reaches max capacity),
> packet is delivered and forwarded, even if NAPI handler is not yet
> complete for the flow.
Have you actually measured this? The latency added by GRO is pure
processing overhead. This is tiny when compared to the time NAPI takes
to wait.
> Say you have 1 us per MSS, then filling 45 MSS per skb means we add a 45
> us delay transit, instead of 16 us, or 1 us if no GRO is used on the
> router.
Are you talking about the latency added by the TX qdisc? That is
not GRO's fault. Perhaps we can add more metadata to the GRO packet
so that the TX qdisc can deal with it appropriately?
> > But please give me chance to have a look first before we give up and
> > install a permanent user-space toggle.
>
> I don't think I ever said it was permanent, I am sorry you understood
> this.
>
> I will be happy to change skb_segment() in the future, but I already
> said I would not expect doing so for linux-3.13, given we were too late
> in the linux-3.12-rc.
For the time being my preference is for your linearisation patch, followed
by a revert of the GRO patch, and lastly the magic toggle that turns this
off for forwarding systems.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH v3 net-next] net: introduce dev_set_forwarding()
From: Eric Dumazet @ 2013-11-04 4:23 UTC (permalink / raw)
To: Herbert Xu
Cc: Ben Hutchings, David Miller, christoph.paasch, netdev, hkchu,
mwdalton
In-Reply-To: <20131104041108.GA22823@gondor.apana.org.au>
On Mon, 2013-11-04 at 12:11 +0800, Herbert Xu wrote:
> On Sun, Nov 03, 2013 at 09:26:43AM -0800, Eric Dumazet wrote:
> >
> > Not really.
> >
> > Have you took a look at the GSO path recently ?
> >
> > The days it was handling only IP+TCP are gone.
> >
> > If you think you can do better, please do so.
>
> OK maybe I overreacted.
>
> With regards to your point 2), GRO does not introduce any latencies
> because it simply relies on NAPI to do the aggregation. IOW it is
> no better or worse latency-wise compared to NAPI. If you need to
> tune it, just use the usual NAPI toggles.
>
Well, GRO adds latencies for sure.
You seem to assume the transmit only can happen when NAPI is done, but
its not true. As soon as GRO fills one packet (reaches max capacity),
packet is delivered and forwarded, even if NAPI handler is not yet
complete for the flow.
Say you have 1 us per MSS, then filling 45 MSS per skb means we add a 45
us delay transit, instead of 16 us, or 1 us if no GRO is used on the
router.
> With repsect to point 3), sure we can allow the generation of TSO
> segments in skb_segment.
>
> You may be right that this is all too hard, since I haven't actually
> sat down and tried to do it yet.
>
> But please give me chance to have a look first before we give up and
> install a permanent user-space toggle.
I don't think I ever said it was permanent, I am sorry you understood
this.
I will be happy to change skb_segment() in the future, but I already
said I would not expect doing so for linux-3.13, given we were too late
in the linux-3.12-rc.
Thanks
^ permalink raw reply
* Re: [PATCH v6] net/hsr: Add support for the High-availability Seamless Redundancy protocol (HSRv0)
From: David Miller @ 2013-11-04 4:20 UTC (permalink / raw)
To: arvid.brodin
Cc: netdev, shemminger, joe, jboticario, balferreira, elias.molina
In-Reply-To: <527167C7.4030305@xdin.com>
From: Arvid Brodin <arvid.brodin@xdin.com>
Date: Wed, 30 Oct 2013 21:10:47 +0100
> High-availability Seamless Redundancy ("HSR") provides instant failover
> redundancy for Ethernet networks. It requires a special network topology where
> all nodes are connected in a ring (each node having two physical network
> interfaces). It is suited for applications that demand high availability and
> very short reaction time.
>
> HSR acts on the Ethernet layer, using a registered Ethernet protocol type to
> send special HSR frames in both directions over the ring. The driver creates
> virtual network interfaces that can be used just like any ordinary Linux
> network interface, for IP/TCP/UDP traffic etc. All nodes in the network ring
> must be HSR capable.
>
> This code is a "best effort" to comply with the HSR standard as described in
> IEC 62439-3:2010 (HSRv0).
>
> Signed-off-by: Arvid Brodin <arvid.brodin@xdin.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH v2 net-next] net: extend net_device allocation to vmalloc()
From: David Miller @ 2013-11-04 4:19 UTC (permalink / raw)
To: eric.dumazet
Cc: joby.poriyath, bhutchings, netdev, wei.liu2, ian.campbell,
xen-devel, andrew.bennieston, david.vrabel, malcolm.crossley
In-Reply-To: <1383163844.4857.59.camel@edumazet-glaptop.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 30 Oct 2013 13:10:44 -0700
> From: Eric Dumazet <edumazet@google.com>
>
> Joby Poriyath provided a xen-netback patch to reduce the size of
> xenvif structure as some netdev allocation could fail under
> memory pressure/fragmentation.
>
> This patch is handling the problem at the core level, allowing
> any netdev structures to use vmalloc() if kmalloc() failed.
>
> As vmalloc() adds overhead on a critical network path, add __GFP_REPEAT
> to kzalloc() flags to do this fallback only when really needed.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: Joby Poriyath <joby.poriyath@citrix.com>
> Cc: Ben Hutchings <bhutchings@solarflare.com>
> ---
> v2: change the Documentation to point to alloc_netdev_mqs()
> instead of kmalloc()/vzalloc(), from Ben
Looks good, applied, thanks Eric.
^ permalink raw reply
* Re: [PATCH ] net_sched: actions - Add default lookup
From: David Miller @ 2013-11-04 4:17 UTC (permalink / raw)
To: eric.dumazet; +Cc: jhs, netdev, ebiederm, alexander.h.duyck
In-Reply-To: <20131103.231232.1438716028901698864.davem@davemloft.net>
From: David Miller <davem@davemloft.net>
Date: Sun, 03 Nov 2013 23:12:32 -0500 (EST)
> Right, and BUG() if we try to register and action with a NULL .lookup
> member.
I return an error, BUG() is too harsh.
^ permalink raw reply
* Re: [PATCH ] net_sched: actions - Add default lookup
From: David Miller @ 2013-11-04 4:12 UTC (permalink / raw)
To: eric.dumazet; +Cc: jhs, netdev, ebiederm, alexander.h.duyck
In-Reply-To: <1383141605.4857.38.camel@edumazet-glaptop.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 30 Oct 2013 07:00:05 -0700
> On Wed, 2013-10-30 at 07:25 -0400, Jamal Hadi Salim wrote:
>> Attached. Tested with simple action.
>>
>> cheers,
>> jamal
>
> Why not setting .lookup to tcf_hash_search
> in the few actions not already doing that ?
>
> This would be more consistent.
> # git grep -n tcf_hash_search
> include/net/act_api.h:92:int tcf_hash_search(struct tc_action *a, u32 index);
> net/sched/act_api.c:198:int tcf_hash_search(struct tc_action *a, u32 index)
> net/sched/act_api.c:209:EXPORT_SYMBOL(tcf_hash_search);
> net/sched/act_csum.c:588: .lookup = tcf_hash_search,
> net/sched/act_gact.c:209: .lookup = tcf_hash_search,
> net/sched/act_ipt.c:301: .lookup = tcf_hash_search,
> net/sched/act_ipt.c:315: .lookup = tcf_hash_search,
> net/sched/act_mirred.c:274: .lookup = tcf_hash_search,
> net/sched/act_nat.c:311: .lookup = tcf_hash_search,
> net/sched/act_pedit.c:246: .lookup = tcf_hash_search,
> net/sched/act_police.c:410: .lookup = tcf_hash_search,
Right, and BUG() if we try to register and action with a NULL .lookup
member.
^ 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