Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH 1/3] netdev: introduce new NETIF_F_HW_SWITCH_OFFLOAD feature flag for switch device offloads
From: Roopa Prabhu @ 2014-12-05  4:17 UTC (permalink / raw)
  To: Jianhua Xie
  Cc: jiri, sfeldma, jhs, bcrl, tgraf, john.fastabend, stephen,
	linville, nhorman, nicolas.dichtel, vyasevic, f.fainelli, buytenh,
	aviadr, netdev, davem, shm, gospo
In-Reply-To: <5481249E.7020902@freescale.com>

On 12/4/14, 7:21 PM, Jianhua Xie wrote:
>
> 在 2014年12月05日 10:26, roopa@cumulusnetworks.com 写道:
>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>
>> This is a generic high level feature flag for all switch asic 
>> features today.
>>
>> switch drivers set this flag on switch ports. Logical devices like
>> bridge, bonds, vxlans can inherit this flag from their slaves/ports.
>>
>> I had to use SWITCH in the name to avoid ambiguity with other feature
>> flags. But, since i have been harping about not calling it 'switch',
>> I am welcome to any suggestions :)
>>
>> An alternative to using a feature flag is to use a IFF_HW_OFFLOAD
>> in net_device_flags.
>> ---
>>   include/linux/netdev_features.h |    2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/include/linux/netdev_features.h 
>> b/include/linux/netdev_features.h
>> index 8e30685..68db1de 100644
>> --- a/include/linux/netdev_features.h
>> +++ b/include/linux/netdev_features.h
>> @@ -66,6 +66,7 @@ enum {
>>       NETIF_F_HW_VLAN_STAG_FILTER_BIT,/* Receive filtering on VLAN 
>> STAGs */
>>       NETIF_F_HW_L2FW_DOFFLOAD_BIT,    /* Allow L2 Forwarding in 
>> Hardware */
>>       NETIF_F_BUSY_POLL_BIT,        /* Busy poll */
>> +    NETIF_F_HW_SWITCH_OFFLOAD_BIT,  /* HW switch offload */
> I am interested in this flag very much, but I am not very clear
> how many offload capabilities does this flag imply.  If this flag
> belongs to a general flag and can be accepted by all vendors,
> I will reuse this flag to introduce another out going data traffics
> distribution offload method to bonding driver.
Right now its a one global offload flag for switch asics. Some may not 
want to tune it more. But others may and so i do expect more flags in 
this category.

Thanks,
Roopa

^ permalink raw reply

* Re: [PATCH 3/3] rocker: set feature NETIF_F_HW_SWITCH_OFFLOAD
From: Roopa Prabhu @ 2014-12-05  4:14 UTC (permalink / raw)
  To: Jianhua Xie
  Cc: jiri, sfeldma, jhs, bcrl, tgraf, john.fastabend, stephen,
	linville, nhorman, nicolas.dichtel, vyasevic, f.fainelli, buytenh,
	aviadr, netdev, davem, shm, gospo
In-Reply-To: <5481259C.3040707@freescale.com>

On 12/4/14, 7:25 PM, Jianhua Xie wrote:
>
> 在 2014年12月05日 10:26, roopa@cumulusnetworks.com 写道:
>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>
>> This patch just sets the feature flag on rocker ports
>> ---
>>   drivers/net/ethernet/rocker/rocker.c |    3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/rocker/rocker.c 
>> b/drivers/net/ethernet/rocker/rocker.c
>> index fded127..3fe19b0 100644
>> --- a/drivers/net/ethernet/rocker/rocker.c
>> +++ b/drivers/net/ethernet/rocker/rocker.c
>> @@ -4003,7 +4003,8 @@ static int rocker_probe_port(struct rocker 
>> *rocker, unsigned int port_number)
>>                  NAPI_POLL_WEIGHT);
>>       rocker_carrier_init(rocker_port);
>>   -    dev->features |= NETIF_F_HW_VLAN_CTAG_FILTER;
>> +    dev->features |= NETIF_F_HW_VLAN_CTAG_FILTER |
>> +                     NETIF_F_HW_SWITCH_OFFLOAD;
> Do you have a plan on enabling/disabling  this flag dynamically by 
> ethtool?

I have not thought about it yet. But if this gets accepted, then yes, I 
will have an ethtool way for drivers who want it.
if we have a IFF_ flag for this (which is also one of my alternate 
proposals), there will be a netlink way to set it on and off.

^ permalink raw reply

* [RFC PATCH net-next] tun: support retrieving multiple packets in a single read with IFF_MULTI_READ
From: Alex Gartrell @ 2014-12-05  4:00 UTC (permalink / raw)
  To: jasonwang
  Cc: davem, netdev, linux-kernel, mst, herbert, kernel-team,
	Alex Gartrell

This patch adds the IFF_MULTI_READ flag.  This has the following behavior.

1) If a read is too short for a packet, a single stripped packet will be read

2) If a read is long enough for multiple packets, as many *full* packets
will be read as possible.  We will not return a stripped packet, so even if
there are many, many packets, we may get a short read.

In casual performance testing with a simple test program that simply reads
and counts packets, IFF_MULTI_READ conservatively yielded a 30% CPU win, as
measured by top.  Load was being driven by a bunch of hpings running on a
server on the same L2 network (single hop through a top-of-rack switch).

Signed-off-by: Alex Gartrell <agartrell@fb.com>
---
 drivers/net/tun.c           | 66 ++++++++++++++++++++++++++++++++++++++-------
 include/uapi/linux/if_tun.h |  3 +++
 2 files changed, 60 insertions(+), 9 deletions(-)

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 6d44da1..f57d618 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -1228,6 +1228,26 @@ static ssize_t tun_chr_write_iter(struct kiocb *iocb, struct iov_iter *from)
 	return result;
 }
 
+static inline size_t tun_calc_max_put_len(const struct tun_struct *tun)
+{
+	size_t len = 0;
+
+	/* It's a pain to peek the skb, so let's assume the worst:
+	 * 1) That skb->len = mtu
+	 * 2) That there is a vlan_tx_tag present
+	 */
+
+	len += tun->dev->mtu + VLAN_HLEN;
+
+	if (tun->flags & TUN_VNET_HDR)
+		len += tun->vnet_hdr_sz;
+
+	if (!(tun->flags & TUN_NO_PI))
+		len += sizeof(struct tun_pi);
+
+	return len;
+}
+
 /* Put packet to the user space buffer */
 static ssize_t tun_put_user(struct tun_struct *tun,
 			    struct tun_file *tfile,
@@ -1343,8 +1363,10 @@ static ssize_t tun_do_read(struct tun_struct *tun, struct tun_file *tfile,
 			   struct iov_iter *to,
 			   int noblock)
 {
+	const size_t max_put_len = tun_calc_max_put_len(tun);
 	struct sk_buff *skb;
-	ssize_t ret;
+	ssize_t ret = 0;
+	ssize_t put_ret = 0;
 	int peeked, err, off = 0;
 
 	tun_debug(KERN_INFO, tun, "tun_do_read\n");
@@ -1355,14 +1377,31 @@ static ssize_t tun_do_read(struct tun_struct *tun, struct tun_file *tfile,
 	if (tun->dev->reg_state != NETREG_REGISTERED)
 		return -EIO;
 
-	/* Read frames from queue */
-	skb = __skb_recv_datagram(tfile->socket.sk, noblock ? MSG_DONTWAIT : 0,
-				  &peeked, &off, &err);
-	if (!skb)
-		return 0;
+	while (!ret || ((tun->flags & TUN_MULTI_READ) &&
+			iov_iter_count(to) >= max_put_len)) {
+		/* Read frames from queue */
+		skb = __skb_recv_datagram(tfile->socket.sk,
+					  noblock ? MSG_DONTWAIT : 0,
+					  &peeked, &off, &err);
+		if (skb) {
+			put_ret = tun_put_user(tun, tfile, skb, to);
+			kfree_skb(skb);
+			if (put_ret < 0) {
+				ret = put_ret;
+				break;
+			}
+			ret += put_ret;
+		} else {
+			if (!ret)
+				ret = err;
+			break;
+		}
 
-	ret = tun_put_user(tun, tfile, skb, to);
-	kfree_skb(skb);
+		/* Now that we've received a datagram, noblock for the
+		 * rest
+		 */
+		noblock = 1;
+	}
 
 	return ret;
 }
@@ -1537,6 +1576,9 @@ static int tun_flags(struct tun_struct *tun)
 	if (tun->flags & TUN_PERSIST)
 		flags |= IFF_PERSIST;
 
+	if (tun->flags & TUN_MULTI_READ)
+		flags |= IFF_MULTI_READ;
+
 	return flags;
 }
 
@@ -1720,6 +1762,11 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
 	else
 		tun->flags &= ~TUN_TAP_MQ;
 
+	if (ifr->ifr_flags & IFF_MULTI_READ)
+		tun->flags |= TUN_MULTI_READ;
+	else
+		tun->flags &= ~TUN_MULTI_READ;
+
 	/* Make sure persistent devices do not get stuck in
 	 * xoff state.
 	 */
@@ -1883,7 +1930,8 @@ static long __tun_chr_ioctl(struct file *file, unsigned int cmd,
 		 * This is needed because we never checked for invalid flags on
 		 * TUNSETIFF. */
 		return put_user(IFF_TUN | IFF_TAP | IFF_NO_PI | IFF_ONE_QUEUE |
-				IFF_VNET_HDR | IFF_MULTI_QUEUE,
+				IFF_VNET_HDR | IFF_MULTI_QUEUE |
+				IFF_MULTI_READ,
 				(unsigned int __user*)argp);
 	} else if (cmd == TUNSETQUEUE)
 		return tun_set_queue(file, &ifr);
diff --git a/include/uapi/linux/if_tun.h b/include/uapi/linux/if_tun.h
index e9502dd..aaf9ddc 100644
--- a/include/uapi/linux/if_tun.h
+++ b/include/uapi/linux/if_tun.h
@@ -36,6 +36,7 @@
 #define TUN_PERSIST 	0x0100	
 #define TUN_VNET_HDR 	0x0200
 #define TUN_TAP_MQ      0x0400
+#define TUN_MULTI_READ	0x0800
 
 /* Ioctl defines */
 #define TUNSETNOCSUM  _IOW('T', 200, int) 
@@ -74,6 +75,8 @@
 #define IFF_PERSIST	0x0800
 #define IFF_NOFILTER	0x1000
 
+#define IFF_MULTI_READ	0x2000
+
 /* Socket options */
 #define TUN_TX_TIMESTAMP 1
 
-- 
Alex Gartrell <agartrell@fb.com>

^ permalink raw reply related

* Re: [PATCH 3/3] rocker: set feature NETIF_F_HW_SWITCH_OFFLOAD
From: Jianhua Xie @ 2014-12-05  3:25 UTC (permalink / raw)
  To: roopa, jiri, sfeldma, jhs, bcrl, tgraf, john.fastabend, stephen,
	linville, nhorman, nicolas.dichtel, vyasevic, f.fainelli, buytenh,
	aviadr
  Cc: netdev, davem, shm, gospo, jianhua Xie
In-Reply-To: <1417746401-8140-4-git-send-email-roopa@cumulusnetworks.com>


在 2014年12月05日 10:26, roopa@cumulusnetworks.com 写道:
> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>
> This patch just sets the feature flag on rocker ports
> ---
>   drivers/net/ethernet/rocker/rocker.c |    3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/rocker/rocker.c b/drivers/net/ethernet/rocker/rocker.c
> index fded127..3fe19b0 100644
> --- a/drivers/net/ethernet/rocker/rocker.c
> +++ b/drivers/net/ethernet/rocker/rocker.c
> @@ -4003,7 +4003,8 @@ static int rocker_probe_port(struct rocker *rocker, unsigned int port_number)
>   		       NAPI_POLL_WEIGHT);
>   	rocker_carrier_init(rocker_port);
>   
> -	dev->features |= NETIF_F_HW_VLAN_CTAG_FILTER;
> +	dev->features |= NETIF_F_HW_VLAN_CTAG_FILTER |
> +					 NETIF_F_HW_SWITCH_OFFLOAD;
Do you have a plan on enabling/disabling  this flag dynamically by ethtool?

Thanks & B.R
Jianhua
>   
>   	err = register_netdev(dev);
>   	if (err) {

^ permalink raw reply

* Re: [PATCH 1/3] netdev: introduce new NETIF_F_HW_SWITCH_OFFLOAD feature flag for switch device offloads
From: Jianhua Xie @ 2014-12-05  3:21 UTC (permalink / raw)
  To: roopa, jiri, sfeldma, jhs, bcrl, tgraf, john.fastabend, stephen,
	linville, nhorman, nicolas.dichtel, vyasevic, f.fainelli, buytenh,
	aviadr
  Cc: netdev, davem, shm, gospo, Jianhua Xie
In-Reply-To: <1417746401-8140-2-git-send-email-roopa@cumulusnetworks.com>


在 2014年12月05日 10:26, roopa@cumulusnetworks.com 写道:
> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>
> This is a generic high level feature flag for all switch asic features today.
>
> switch drivers set this flag on switch ports. Logical devices like
> bridge, bonds, vxlans can inherit this flag from their slaves/ports.
>
> I had to use SWITCH in the name to avoid ambiguity with other feature
> flags. But, since i have been harping about not calling it 'switch',
> I am welcome to any suggestions :)
>
> An alternative to using a feature flag is to use a IFF_HW_OFFLOAD
> in net_device_flags.
> ---
>   include/linux/netdev_features.h |    2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h
> index 8e30685..68db1de 100644
> --- a/include/linux/netdev_features.h
> +++ b/include/linux/netdev_features.h
> @@ -66,6 +66,7 @@ enum {
>   	NETIF_F_HW_VLAN_STAG_FILTER_BIT,/* Receive filtering on VLAN STAGs */
>   	NETIF_F_HW_L2FW_DOFFLOAD_BIT,	/* Allow L2 Forwarding in Hardware */
>   	NETIF_F_BUSY_POLL_BIT,		/* Busy poll */
> +	NETIF_F_HW_SWITCH_OFFLOAD_BIT,  /* HW switch offload */
I am interested in this flag very much, but I am not very clear
how many offload capabilities does this flag imply.  If this flag
belongs to a general flag and can be accepted by all vendors,
I will reuse this flag to introduce another out going data traffics
distribution offload method to bonding driver.

Thanks & B.R.
Jianhua
>   
>   	/*
>   	 * Add your fresh new feature above and remember to update
> @@ -124,6 +125,7 @@ enum {
>   #define NETIF_F_HW_VLAN_STAG_TX	__NETIF_F(HW_VLAN_STAG_TX)
>   #define NETIF_F_HW_L2FW_DOFFLOAD	__NETIF_F(HW_L2FW_DOFFLOAD)
>   #define NETIF_F_BUSY_POLL	__NETIF_F(BUSY_POLL)
> +#define NETIF_F_HW_SWITCH_OFFLOAD	__NETIF_F(HW_SWITCH_OFFLOAD)
>   
>   /* Features valid for ethtool to change */
>   /* = all defined minus driver/device-class-related */

^ permalink raw reply

* Re: [patch iproute2 0/6] iproute2: add changes for switchdev
From: Roopa Prabhu @ 2014-12-05  2:28 UTC (permalink / raw)
  To: Scott Feldman
  Cc: Jiri Pirko, Netdev, David S. Miller, nhorman@tuxdriver.com,
	Andy Gospodarek, Thomas Graf, dborkman@redhat.com,
	ogerlitz@mellanox.com, jesse@nicira.com, pshelar@nicira.com,
	azhou@nicira.com, ben@decadent.org.uk, stephen@networkplumber.org,
	Kirsher, Jeffrey T, vyasevic@redhat.com, Cong Wang,
	Fastabend, John R, Eric Dumazet, Jamal Hadi Salim,
	Florian Fainelli, John Linville
In-Reply-To: <CAE4R7bCyf7VBFWqEme37rf0YbTc3-s5Qka+1HMOsdMgVC3qZ-A@mail.gmail.com>

On 12/4/14, 12:49 PM, Scott Feldman wrote:
> On Thu, Dec 4, 2014 at 8:55 AM, Roopa Prabhu <roopa@cumulusnetworks.com> wrote:
>> On 12/4/14, 8:04 AM, Jiri Pirko wrote:
>>> Thu, Dec 04, 2014 at 03:45:44PM CET, roopa@cumulusnetworks.com wrote:
>>>> On 12/4/14, 6:34 AM, Jiri Pirko wrote:
>>>>> Thu, Dec 04, 2014 at 03:26:50PM CET, roopa@cumulusnetworks.com wrote:
>>>>>> On 12/4/14, 12:57 AM, Jiri Pirko wrote:
>>>>>>> Jiri Pirko (1):
>>>>>>>     iproute2: ipa: show switch id
>>>>>>>
>>>>>>> Scott Feldman (5):
>>>>>>>     bridge/fdb: fix statistics output spacing
>>>>>>>     bridge/fdb: add flag/indication for FDB entry synced from offload
>>>>>>>       device
>>>>>>>     bridge/link: add new offload hwmode swdev
>>>>>> Ack to most patches but nack on this one. The todo list still has a
>>>>>> note to
>>>>>> revist the flag to indicate switchdev offloads.
>>>>>> Exposing this to userspace does not help that.
>>>>> Hmm, note that this is already exposed to userspace, this patchset is
>>>>> for iproute2 (userspace tool).
>>>> hmmm, all feedback on the switchdev patches seemed to indicate we can
>>>> change
>>>> this later.
>>>> I don't see swdev mode being used in the kernel anywhere today.
>>> Well, it is, in rocker:
>>> $ git grep BRIDGE_MODE_SWDEV
>>> drivers/net/ethernet/rocker/rocker.c:                   if (mode !=
>>> BRIDGE_MODE_SWDEV)
>>> drivers/net/ethernet/rocker/rocker.c:   u16 mode = BRIDGE_MODE_SWDEV;
>>> include/uapi/linux/if_bridge.h:#define BRIDGE_MODE_SWDEV        2       /*
>>> Full switch device offload */
>>
>> The problem is rocker is not the only one who is going to be using this. And
>> so, we need something that fits everybody.
>> And i am not going to make my user set a mode for him to enable offload to
>> hw.
>>
>>>> I will send a patch to remove it. Its still in net-next and so can be
>>>> changed
>>>> ?.
>>>> I was going to resend my patch to introduce a common offload flag for all
>>>> link objects.
>>>> It would be nice if all of them had a consistent flag to indicate hw
>>>> offload
>>>> and iproute2 could display the same flag for all.
>>>> Including bonds and vxlan's.
>>> I do not understand the connection with BRIDGE_MODE_SWDEV. We discussed
>>> this already. BRIDGE_MODE_SWDEV is a bridge mode, similar to for example
>>> BRIDGE_MODE_VEPA and makes perfect sense to have it.
>> I dont think everybody acked it. But it went in with a note saying that it
>> can be changed.
> I thought that was the plan: this new mode goes in now for net-next
> and iproute2, and you would supply follow up patch for each to move to
> your switch port flag.  That will give us time to review your work
> without have net-next and iproute2 out-of-sync.
>
>>>
>>> How vxlan and bonds come into the mixture, that is a puzzler for me.
>>> Maybe I have to see patches.
>>
>> I had posted a version of the patch previously:
>> http://www.spinics.net/lists/netdev/msg305472.html
>>
>> I have a v2 patch in my stack which does not touch the netlink header.
>> But in the past hour, i have been thinking about it some more. Do we really
>> need this set by the user ?. In my use case i don't need it.
> Look at how iproute2 figures out if SELF should be set or not.  It's
> only set if hwmode is set, otherwise it defaults to MASTER.  So with
> SWDEV a new hwmode, we can push settings (learning, leraning_sync) to
> port with SELF set.  It's probably not an ideal arrangement having to
> set hwmode each time, but this was the low-touch change to iproute2 to
> push port settings.
Did not know about this. Thanks for the info.
>
> I'm hoping your new patch will kind of straighten this all out.  But
> you've got extra work to make sure backward compat with older iproute2
> still works, including this weirdness around hwmode.
>
>> We do need a feature flag (or net_device_flags), but it does not need to be
>> set by the user explicitly.
>> This flag can be set by the switch port driver on the switch ports. And the
>> logical device: bridge/bond/vxlan
>> can inherit it from the port. There was a need of a flag in some usecases,
>> to control offloading of specific bridge port flags
>> to hw/sw (example learning in hw or sw). example patch:
>> https://patchwork.ozlabs.org/patch/413211/
>>
>> I will post something today.
> Can you include matching iproute2 changes?  (Assuming you'll building
> on top of what's already in net-next and this iproute2 set Jiri sent
> out).  It's helpful to see the iproute2 changes to see what the new
> cmd structure is and how legacy is handled.
Just sent what ever i had. Take a look and let me know.

Thanks,
Roopa


>

^ permalink raw reply

* [PATCH iproute2] bridge link: add option 'self'
From: roopa @ 2014-12-05  2:27 UTC (permalink / raw)
  To: jiri, sfeldma, jhs, bcrl, tgraf, john.fastabend, stephen,
	linville, nhorman, nicolas.dichtel, vyasevic, f.fainelli, buytenh,
	aviadr
  Cc: netdev, davem, shm, gospo, Roopa Prabhu

From: Roopa Prabhu <roopa@cumulusnetworks.com>

Currently self is set internally only if hwmode is set.
This makes it necessary for the hw to have a mode.
There is no hwmode really required to go to hardware. So, introduce
self for anybody who wants to target hardware.
---
 bridge/link.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/bridge/link.c b/bridge/link.c
index 90d9e7f..2b86141 100644
--- a/bridge/link.c
+++ b/bridge/link.c
@@ -321,6 +321,9 @@ static int brlink_modify(int argc, char **argv)
 					"\"veb\".\n");
 				exit(-1);
 			}
+		} else if (strcmp(*argv, "self") == 0) {
+			NEXT_ARG();
+			flags = BRIDGE_FLAGS_SELF;
 		} else {
 			usage();
 		}
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH 3/3] rocker: set feature NETIF_F_HW_SWITCH_OFFLOAD
From: roopa @ 2014-12-05  2:26 UTC (permalink / raw)
  To: jiri, sfeldma, jhs, bcrl, tgraf, john.fastabend, stephen,
	linville, nhorman, nicolas.dichtel, vyasevic, f.fainelli, buytenh,
	aviadr
  Cc: netdev, davem, shm, gospo, Roopa Prabhu

From: Roopa Prabhu <roopa@cumulusnetworks.com>

This patch just sets the feature flag on rocker ports
---
 drivers/net/ethernet/rocker/rocker.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/rocker/rocker.c b/drivers/net/ethernet/rocker/rocker.c
index fded127..3fe19b0 100644
--- a/drivers/net/ethernet/rocker/rocker.c
+++ b/drivers/net/ethernet/rocker/rocker.c
@@ -4003,7 +4003,8 @@ static int rocker_probe_port(struct rocker *rocker, unsigned int port_number)
 		       NAPI_POLL_WEIGHT);
 	rocker_carrier_init(rocker_port);
 
-	dev->features |= NETIF_F_HW_VLAN_CTAG_FILTER;
+	dev->features |= NETIF_F_HW_VLAN_CTAG_FILTER |
+					 NETIF_F_HW_SWITCH_OFFLOAD;
 
 	err = register_netdev(dev);
 	if (err) {
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH 2/3] bridge: offload bridge port attributes to switch asic if feature flag set
From: roopa @ 2014-12-05  2:26 UTC (permalink / raw)
  To: jiri, sfeldma, jhs, bcrl, tgraf, john.fastabend, stephen,
	linville, nhorman, nicolas.dichtel, vyasevic, f.fainelli, buytenh,
	aviadr
  Cc: netdev, davem, shm, gospo, Roopa Prabhu

From: Roopa Prabhu <roopa@cumulusnetworks.com>

This allows offloading to switch asic without having the user to set
any flag. And this is done in the bridge driver to rollback kernel settings
on hw offload failure if required in the future.

With this, it also makes sure a notification goes out only after the
attributes are set both in the kernel and hw.
---
 net/bridge/br_netlink.c |   27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 9f5eb55..ce173f0 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -407,9 +407,21 @@ int br_setlink(struct net_device *dev, struct nlmsghdr *nlh)
 				afspec, RTM_SETLINK);
 	}
 
+	if ((dev->features & NETIF_F_HW_SWITCH_OFFLOAD) &&
+			dev->netdev_ops->ndo_bridge_setlink) {
+		int ret = dev->netdev_ops->ndo_bridge_setlink(dev, nlh);
+		if (ret && ret != -EOPNOTSUPP) {
+			/* XXX Fix this in the future to rollback
+			 * kernel settings and return error
+			 */
+			br_warn(p->br, "error offloading bridge attributes " 
+					"on port %u(%s)\n", (unsigned int) p->port_no,
+					p->dev->name);
+		}
+	}
+
 	if (err == 0)
 		br_ifinfo_notify(RTM_NEWLINK, p);
-
 out:
 	return err;
 }
@@ -433,6 +445,19 @@ int br_dellink(struct net_device *dev, struct nlmsghdr *nlh)
 	err = br_afspec((struct net_bridge *)netdev_priv(dev), p,
 			afspec, RTM_DELLINK);
 
+	if (dev->features & NETIF_F_HW_SWITCH_OFFLOAD
+			&& dev->netdev_ops->ndo_bridge_setlink) {
+		int ret = dev->netdev_ops->ndo_bridge_dellink(dev, nlh);
+		if (ret && ret != -EOPNOTSUPP) {
+			/* XXX Fix this in the future to rollback
+			 * kernel settings and return error
+			 */
+			br_warn(p->br, "error offloading bridge attributes " 
+					"on port %u(%s)\n", (unsigned int) p->port_no,
+					p->dev->name);
+		}
+	}
+
 	return err;
 }
 static int br_validate(struct nlattr *tb[], struct nlattr *data[])
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH 1/3] netdev: introduce new NETIF_F_HW_SWITCH_OFFLOAD feature flag for switch device offloads
From: roopa @ 2014-12-05  2:26 UTC (permalink / raw)
  To: jiri, sfeldma, jhs, bcrl, tgraf, john.fastabend, stephen,
	linville, nhorman, nicolas.dichtel, vyasevic, f.fainelli, buytenh,
	aviadr
  Cc: netdev, davem, shm, gospo, Roopa Prabhu

From: Roopa Prabhu <roopa@cumulusnetworks.com>

This is a generic high level feature flag for all switch asic features today.

switch drivers set this flag on switch ports. Logical devices like
bridge, bonds, vxlans can inherit this flag from their slaves/ports.

I had to use SWITCH in the name to avoid ambiguity with other feature
flags. But, since i have been harping about not calling it 'switch',
I am welcome to any suggestions :)

An alternative to using a feature flag is to use a IFF_HW_OFFLOAD
in net_device_flags.
---
 include/linux/netdev_features.h |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h
index 8e30685..68db1de 100644
--- a/include/linux/netdev_features.h
+++ b/include/linux/netdev_features.h
@@ -66,6 +66,7 @@ enum {
 	NETIF_F_HW_VLAN_STAG_FILTER_BIT,/* Receive filtering on VLAN STAGs */
 	NETIF_F_HW_L2FW_DOFFLOAD_BIT,	/* Allow L2 Forwarding in Hardware */
 	NETIF_F_BUSY_POLL_BIT,		/* Busy poll */
+	NETIF_F_HW_SWITCH_OFFLOAD_BIT,  /* HW switch offload */
 
 	/*
 	 * Add your fresh new feature above and remember to update
@@ -124,6 +125,7 @@ enum {
 #define NETIF_F_HW_VLAN_STAG_TX	__NETIF_F(HW_VLAN_STAG_TX)
 #define NETIF_F_HW_L2FW_DOFFLOAD	__NETIF_F(HW_L2FW_DOFFLOAD)
 #define NETIF_F_BUSY_POLL	__NETIF_F(BUSY_POLL)
+#define NETIF_F_HW_SWITCH_OFFLOAD	__NETIF_F(HW_SWITCH_OFFLOAD)
 
 /* Features valid for ethtool to change */
 /* = all defined minus driver/device-class-related */
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH net-next 0/3] switchdev offload flags
From: roopa @ 2014-12-05  2:26 UTC (permalink / raw)
  To: jiri, sfeldma, jhs, bcrl, tgraf, john.fastabend, stephen,
	linville, nhorman, nicolas.dichtel, vyasevic, f.fainelli, buytenh,
	aviadr
  Cc: netdev, davem, shm, gospo, Roopa Prabhu

From: Roopa Prabhu <roopa@cumulusnetworks.com>

This patch series only addresses bridge link attribute offloads to hardware.

It is a continuation of my previous series on switchdev policy attributes:
http://www.spinics.net/lists/netdev/msg305469.html

Looking at the current state of bridge l2 offload in the kernel,
    - flag 'self' is the way to directly manage the bridge device in hw via
      the ndo_bridge_setlink/ndo_bridge_getlink calls
 
    - flag 'master' is always used to manage the in kernel bridge devices
      via the same ndo_bridge_setlink/ndo_bridge_getlink calls

Today these are used separately. The nic offloads use hwmode "vepa/veb" to go
directly to hw with the "self" flag.

At this point i am trying not to introduce any new user facing flags/attributes.

In the model where we want the kernel bridge device to be offloaded to
hardware (In other words, sync kernel bridge state to hw),
we very much want the bridge driver to be involved.

With this patch series,
When the user sends a bridge setlink message, it will come in with 'master',
        - go to the bridge device,
        - set settings in the kernel
        - if offload mode is set on the port, also call the port driver
          offload ndo_bridge_setlink
        If you want to act on the hw alone, you can still use the self flag to
        go to the hw or port driver directly.
        (There is no need to specify a hardware mode to go to the port driver)

(To selectively offload bridge port attributes,
example learning in hw only etc, we can introduce offload bits for 
per bridge port flag attribute as in my previous patch
https://patchwork.ozlabs.org/patch/413211/. I have not included that in this
series)


Roopa Prabhu (3):
  netdev: introduce new NETIF_F_HW_SWITCH_OFFLOAD feature flag for
    switch device offloads
  bridge: offload bridge port attributes to switch asic if feature flag
    set
  rocker: set feature NETIF_F_HW_SWITCH_OFFLOAD

 drivers/net/ethernet/rocker/rocker.c |    3 ++-
 include/linux/netdev_features.h      |    2 ++
 net/bridge/br_netlink.c              |   27 ++++++++++++++++++++++++++-
 3 files changed, 30 insertions(+), 2 deletions(-)

-- 
1.7.10.4

^ permalink raw reply

* Re: 3.12.33 - BUG xfrm_selector_match+0x25/0x2f6
From: Smart Weblications GmbH - Florian Wiessner @ 2014-12-05  2:23 UTC (permalink / raw)
  To: Julian Anastasov, Steffen Klassert; +Cc: netdev, LKML, stable
In-Reply-To: <alpine.LFD.2.11.1412042338370.4841@ja.home.ssi.bg>

Hi,

Am 05.12.2014 00:15, schrieb Julian Anastasov:
> 
> 	Hello,
> 
> On Thu, 4 Dec 2014, Steffen Klassert wrote:
> 
>>> [16623.096721] Call Trace:
>>> [16623.096744]  <IRQ>
>>> [16623.096749]  [<ffffffff81547a7c>] ? xfrm_sk_policy_lookup+0x44/0x9b
>>> [16623.096802]  [<ffffffff81547ef7>] ? xfrm_lookup+0x91/0x446
>>> [16623.096832]  [<ffffffff81541316>] ? ip_route_me_harder+0x150/0x1b0
>>> [16623.096865]  [<ffffffffa01b6457>] ? ip_vs_route_me_harder+0x86/0x91 [ip_vs]
>>> [16623.096899]  [<ffffffffa01b797a>] ? ip_vs_out+0x2d3/0x5bc [ip_vs]
>>> [16623.096930]  [<ffffffff81501420>] ? ip_rcv_finish+0x2b8/0x2b8
>>
>> I really wonder why the xfrm_sk_policy_lookup codepath is taken here.
>> It looks like this is the processing of an inbound ipv4 packet that
>> is going to be rerouted to the output path by ipvs, so this packet
>> should not have socket context at all.
> 
> 	In above trace looks like IPVS-NAT is used between
> local client and some real server. IPVS handles this skb
> at LOCAL_IN and calls ip_vs_route_me_harder(). If we have
> skb->sk at LOCAL_IN, my first thought is about early demux.
> 
> 	If I remember correctly, looking at commit f5a41847acc535e2
> ("ipvs: move ip_route_me_harder for ICMP") that introduced
> this rerouting (2.6.37), it was needed because at that time TCP
> used rt_src from received skb to select daddr in ip_send_reply().
> As packets to server are DNAT-ed and packets to client are
> SNAT-ed we used rerouting to fill rt_src with correct IP
> after SNAT.
> 
> 	Now when routing cache is removed in 3.6 and
> tcp_v4_send_reset() is changed to provide ip_hdr(skb)->saddr
> instead of rt_src it should be safe to remove this rerouting,
> it is enough that ip_hdr(skb)->saddr was updated on IPVS-SNAT at
> LOCAL_IN. In fact, rt_src was removed early in 3.0 with
> commit 0a5ebb8000c5362 ("ipv4: Pass explicit daddr arg to 
> ip_send_reply().").
> 
> 	This is only to explain above stack. Not sure
> if problem is related somehow to early demux but such
> commits look interesting:
> 
> - commit 6b8dbcf2c44fd7a ("bridge: netfilter: orphan skb before invoking 
> ip netfilter hooks")
> 
> 	Also, it would be good to know which 3.x kernel between
> 3.13 and 3.17 fixes the problem, it will narrow the search.
> 


i tried with 3.12.33 without any XFRM and now got this one (which is reproducable):

[  233.956012] BUG: unable to handle kernel NULL pointer dereference at 00000000
                                   00000014
[  233.956218] IP: [<ffffffffa013a470>] nf_ct_seqadj_set+0x60/0x90 [nf_conntrack
                                   ]
[  233.956371] PGD 0
[  233.956493] Oops: 0000 [#1] SMP
[  233.956680] Modules linked in: netconsole xt_nat xt_multiport veth iptable_ma
                                   ngle xt_mark nf_conntrack_netlink nfnetlink
ip_vs_rr ipt_MASQUERADE iptable_nat
nf_nat_ipv4 nf_conntrack_ipv4 nf_defrag_ipv4 ipt_REJECT xt_tcpudp iptable_filter
                                    ip_tables cpufreq_ondemand cpufreq_powersave
cpufreq_conservative cpufreq_users                                    pace
ocfs2_stack_o2cb ocfs2_dlm bridge stp llc bonding fuse nf_conntrack_ftp 802
                               1q openvswitch gre vxlan xt_conntrack x_tables
ocfs2_dlmfs dlm sctp ocfs2 ocfs2_                                    nodemanager
ocfs2_stackglue configfs rbd kvm_intel kvm coretemp ip_vs_ftp ip_vs
                        nf_nat nf_conntrack psmouse i2c_i801 serio_raw lpc_ich
mfd_core evdev btrfs lzo_                                    decompress lzo_compress
[  233.960221] CPU: 2 PID: 29996 Comm: vsftpd Not tainted 3.12.33 #4
[  233.960298] Hardware name: Supermicro X9SCI/X9SCA/X9SCI/X9SCA, BIOS 1.1a 09/2
                                   8/2011
[  233.960395] task: ffff88075e87a2c0 ti: ffff8806a7444000 task.ti: ffff8806a744
                                   4000
[  233.960486] RIP: 0010:[<ffffffffa013a470>]  [<ffffffffa013a470>] nf_ct_seqadj
                                   _set+0x60/0x90 [nf_conntrack]
[  233.960632] RSP: 0018:ffff88083fc83998  EFLAGS: 00010206
[  233.960709] RAX: 000000000000000c RBX: ffff8806cab452cc RCX: 0000000000000003
[  233.960791] RDX: 0000000000000029 RSI: 0000000000000003 RDI: ffff8806cab452cc
[  233.960875] RBP: 00000000ee38035a R08: ffff8807e2b1edc0 R09: ffff88083fc839a8
[  233.960957] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000003
[  233.961041] R13: 0000000000000000 R14: 0000000000000003 R15: ffff8806a75a50bc
[  233.961124] FS:  00007ff22daec700(0000) GS:ffff88083fc80000(0000) knlGS:00000
                                   00000000000
[  233.961226] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  233.961303] CR2: 0000000000000014 CR3: 00000006b3259000 CR4: 00000000000407e0
[  233.961384] Stack:
[  233.961460]  ffff880815612b60 0000000000000012 0000000000000014 ffff8806cab45
                                   2c8
[  233.961776]  ffff8806a75a5001 ffffffffa014f681 0000000000000000 ffffffff00000
                                   045
[  233.962095]  ffff880800000048 0000001b00000003 ffff88083fc83a70 ffff880815612
                                   b60
[  233.962411] Call Trace:
[  233.962482]  <IRQ>
[  233.962538]  [<ffffffffa014f681>] ? __nf_nat_mangle_tcp_packet+0x109/0x120 [n
                                   f_nat]
[  233.962762]  [<ffffffffa017749e>] ? ip_vs_ftp_out.part.8+0x2b2/0x338 [ip_vs_f
                                   tp]
[  233.962866]  [<ffffffff814cb8c0>] ? __domain_mapping+0x25d/0x2a3
[  233.962949]  [<ffffffff8154140c>] ? fib_table_lookup+0xe4/0x255
[  233.963032]  [<ffffffffa015f858>] ? ip_vs_app_pkt_out+0x105/0x18b [ip_vs]
[  233.963110]  [<ffffffffa0162ffc>] ? tcp_snat_handler+0x6b/0x320 [ip_vs]
[  233.963189]  [<ffffffffa0155d3d>] ? ip_vs_conn_out_get_proto+0x1c/0x25 [ip_vs
                                   ]
[  233.963284]  [<ffffffffa0158937>] ? ip_vs_out+0x290/0x5bc [ip_vs]
[  233.963362]  [<ffffffff8150f544>] ? ip_frag_mem+0x2a/0x2a
[  233.963442]  [<ffffffff81508e1f>] ? nf_iterate+0x42/0x80
[  233.963519]  [<ffffffff81508ec6>] ? nf_hook_slow+0x69/0xff
[  233.963595]  [<ffffffff8150f544>] ? ip_frag_mem+0x2a/0x2a
[  233.963667]  [<ffffffff8150f8ae>] ? ip_forward+0x22d/0x2cf
[  233.963744]  [<ffffffff814e57ce>] ? __netif_receive_skb_core+0x5f0/0x66c
[  233.963826]  [<ffffffff814e59df>] ? process_backlog+0x13e/0x13e
[  233.963911]  [<ffffffffa0455e09>] ? br_handle_frame_finish+0x382/0x382 [bridg
                                   e]
[  233.964008]  [<ffffffff814e5a2b>] ? netif_receive_skb+0x4c/0x7d
[  233.964090]  [<ffffffffa0455d95>] ? br_handle_frame_finish+0x30e/0x382 [bridg
                                   e]
[  233.964186]  [<ffffffffa0455fda>] ? br_handle_frame+0x1d1/0x217 [bridge]
[  233.964267]  [<ffffffff814e567d>] ? __netif_receive_skb_core+0x49f/0x66c
[  233.964350]  [<ffffffff814e592b>] ? process_backlog+0x8a/0x13e
[  233.964429]  [<ffffffff814e5c31>] ? net_rx_action+0xa2/0x1c0
[  233.964508]  [<ffffffff81047e2e>] ? __do_softirq+0xf6/0x24f
[  233.964588]  [<ffffffff8106cbfd>] ? account_system_time+0x10f/0x169
[  233.964669]  [<ffffffff815ad7dc>] ? call_softirq+0x1c/0x30
[  233.964743]  <EOI>
[  233.964801]  [<ffffffff8100464d>] ? do_softirq+0x2c/0x5f
[  233.965013]  [<ffffffff81047ca1>] ? local_bh_enable+0x67/0x85
[  233.965088]  [<ffffffff81511689>] ? ip_finish_output+0x2c9/0x322
[  233.965165]  [<ffffffff8151240a>] ? ip_queue_xmit+0x2b7/0x2f0
[  233.965239]  [<ffffffff81524772>] ? tcp_transmit_skb+0x6ef/0x755
[  233.965316]  [<ffffffff815250e8>] ? tcp_write_xmit+0x886/0x9cb
[  233.965391]  [<ffffffff8152527a>] ? __tcp_push_pending_frames+0x24/0x7e
[  233.965473]  [<ffffffff8151a33c>] ? tcp_sendmsg+0xa4c/0xbfc
[  233.965550]  [<ffffffff814d3477>] ? sock_aio_write+0xe3/0xfd
[  233.965631]  [<ffffffff81122f4d>] ? do_sync_write+0x59/0x79
[  233.965709]  [<ffffffff811239e3>] ? vfs_write+0xc4/0x182
[  233.965786]  [<ffffffff81123daf>] ? SyS_write+0x45/0x7c
[  233.965864]  [<ffffffff815ac35b>] ? tracesys+0xdd/0xe2
[  233.965940] Code: 68 14 4d 01 c5 45 85 e4 74 46 f0 80 4f 78 40 48 8d 5f 04 48
                                    89 df e8 00 12 47 e1 31 c0 41 83 fe 02 0f 97
c0 48 6b c0 0c 4c 01 e8 <8b> 70 08                                     39 70 04
74 08 89 ea 0f ca 39 10 79 0d 89 70 04 44 01
[  233.969602] RIP  [<ffffffffa013a470>] nf_ct_seqadj_set+0x60/0x90 [nf_conntrac
                                   k]
[  233.969746]  RSP <ffff88083fc83998>
[  233.969816] CR2: 0000000000000014
[  233.969919] ---[ end trace c6faf7aa989b11c2 ]---
[  233.969999] Kernel panic - not syncing: Fatal exception in interrupt
[  233.970081] Rebooting in 10 seconds..
[  244.029931] ACPI MEMORY or I/O RESET_REG.


node01:/ocfs2/usr/src/linux-3.12.33/scripts# ./decodecode < /tmp/oops-ipvsftp.txt
[ 233.965940] Code: 68 14 4d 01 c5 45 85 e4 74 46 f0 80 4f 78 40 48 8d 5f 04 48
89 df e8 00 12 47 e1 31 c0 41 83 fe 02 0f 97 c0 48 6b c0 0c 4c 01 e8 <8b> 70 08
39 70 04 74 08 89 ea 0f ca 39 10 79 0d 89 70 04 44 01
All code
========
   0:   68 14 4d 01 c5          pushq  $0xffffffffc5014d14
   5:   45 85 e4                test   %r12d,%r12d
   8:   74 46                   je     0x50
   a:   f0 80 4f 78 40          lock orb $0x40,0x78(%rdi)
   f:   48 8d 5f 04             lea    0x4(%rdi),%rbx
  13:   48 89 df                mov    %rbx,%rdi
  16:   e8 00 12 47 e1          callq  0xffffffffe147121b
  1b:   31 c0                   xor    %eax,%eax
  1d:   41 83 fe 02             cmp    $0x2,%r14d
  21:   0f 97 c0                seta   %al
  24:   48 6b c0 0c             imul   $0xc,%rax,%rax
  28:   4c 01 e8                add    %r13,%rax
  2b:*  8b 70 08                mov    0x8(%rax),%esi           <-- trapping
instruction
  2e:   39 70 04                cmp    %esi,0x4(%rax)
  31:   74 08                   je     0x3b
  33:   89 ea                   mov    %ebp,%edx
  35:   0f ca                   bswap  %edx
  37:   39 10                   cmp    %edx,(%rax)
  39:   79 0d                   jns    0x48
  3b:   89 70 04                mov    %esi,0x4(%rax)
  3e:   44                      rex.R
  3f:   01                      .byte 0x1

Code starting with the faulting instruction
===========================================
   0:   8b 70 08                mov    0x8(%rax),%esi
   3:   39 70 04                cmp    %esi,0x4(%rax)
   6:   74 08                   je     0x10
   8:   89 ea                   mov    %ebp,%edx
   a:   0f ca                   bswap  %edx
   c:   39 10                   cmp    %edx,(%rax)
   e:   79 0d                   jns    0x1d
  10:   89 70 04                mov    %esi,0x4(%rax)
  13:   44                      rex.R
  14:   01                      .byte 0x1


setup is like this:


#virtual=<myVIP>:21
#       real=10.10.1.20:21 masq
#       real=10.10.1.21:21 masq
#       real=10.10.1.22:21 masq
#       real=10.10.1.23:21 masq
#       persistent=600
#       service=ftp
#       scheduler=rr
#       protocol=tcp
#       checktype=connect

( i remarked it to prevent fruther crashes...)

when ip_vs_ftp is loaded and someone trying to make a ftp connection, the system
panics instantly.

10.10.1.20 - 10.10.1.23 are lxc-containers using veth connected to the bridge
running on 4 different nodes. The node running ldirector/ipvsadm has also one of
those containers running (don't know if that matters)

brctl show
bridge name     bridge id               STP enabled     interfaces
br0             8000.00259052bbf4       no              bond0
                                                        vethMKELUc
                                                        vethXdWGqf
                                                        vethgJMmEb
                                                        vethmKNqFc


I disabled the ftp server lxc container on the node doing ip_vs, so that the
endpoint of the connection is not on the same node and tried again but with the
same result.

Unfortunatelly i cannot test with newer kernels than 3.12, because ocfs2 is
somehow broken in >= 3.14


-- 

Mit freundlichen Grüßen,

Florian Wiessner

Smart Weblications GmbH
Martinsberger Str. 1
D-95119 Naila

fon.: +49 9282 9638 200
fax.: +49 9282 9638 205
24/7: +49 900 144 000 00 - 0,99 EUR/Min*
http://www.smart-weblications.de

--
Sitz der Gesellschaft: Naila
Geschäftsführer: Florian Wiessner
HRB-Nr.: HRB 3840 Amtsgericht Hof
*aus dem dt. Festnetz, ggf. abweichende Preise aus dem Mobilfunknetz

^ permalink raw reply

* Re: Is this 32-bit NCM?y
From: Kevin Zhu @ 2014-12-05  2:20 UTC (permalink / raw)
  To: Enrico Mioso, Bjørn Mork
  Cc: Midge Shaojun  Tan, Eli Britstein, Alex Strizhevsky,
	youtux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <alpine.LNX.2.03.1412041326160.9926-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 4821 bytes --]

Regarding the location of NDP, it should be easy to fix. It can be added
to the end of the NTB only after it's ready to send. Regarding the
concern to other devices, as there's a particular driver for Huawei
devices in kernel, which is huawei_cdc_ncm, maybe we can just fix the TX
function there to avoid breaking other devices.

Regards,
Kevin

On 12/04/2014 08:28 PM, Enrico Mioso wrote:
> ... DHCP will work with some DHCPNACKS in the meanwhile, but ping
> stops working at all.
> Otherwise, it works with the standard value:
>
> --- 8.8.8.8 ping statistics ---
> 48 packets transmitted, 48 received, 0% packet loss, time 47004ms
> rtt min/avg/max/mdev = 362.084/392.878/523.132/33.636 ms
>
> And I was expecting effectively to see some lost packets, but
> instead... no.
>
>
> On Thu, 4 Dec 2014, Bjørn Mork wrote:
>
>> Date: Thu, 4 Dec 2014 12:44:56
>> From: Bjørn Mork <bjorn@mork.no>
>> To: Midge Shaojun Tan <ShaojunMidge.Tan@audiocodes.com>
>> Cc: Enrico Mioso <mrkiko.rs@gmail.com>,
>>     Kevin Zhu <Mingying.Zhu@audiocodes.com>,
>>     Eli Britstein <Eli.Britstein@audiocodes.com>,
>>     Alex Strizhevsky <alexxst@gmail.com>,
>>     "youtux@gmail.com" <youtux@gmail.com>,
>>     "linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
>>     "netdev@vger.kernel.org" <netdev@vger.kernel.org>
>> Subject: Re: Is this 32-bit NCM?y
>>
>> "Midge Shaojun  Tan" <ShaojunMidge.Tan@audiocodes.com> writes:
>>
>>> Hi all,
>>>
>>> I test OK with kervel 3.16.4
>>> Need disable other Ethernet network, just like eth1. (Then the DNS
>>> and route is OK)
>>> And also need disable arp, (ifconfig wwan0 -arp up), because China
>>> UNICOM don't respond the ARP message.
>>
>> The ARP functionality is independent of operator.  It is handled
>> internally by the modem firmware.  There are no MAC addresses or
>> ethernet headers transmitted over the radio link.  That's all faked by
>> the modem.  All MAC addresses and ethernet headers are local to the
>> modem<->host USB link.
>>
>>> With new mode switch string: /etc/usb_modeswitch.d/12d1:14fe
>>> Please see the patch and check whether it is correct?
>>
>> I see that you have two changes there:
>>
>> 1) the ETH_HLEN adjustment of ctx->tx_remainder is dropped
>> 2) the NDP is placed after the first frame.
>>
>> I haven't verified the effect of the tx_remainder change, but I assume
>> it fixes an alignment problem for this device.  I'd like to look more at
>> the effect of this for different values of wNdpOutPayloadRemainder and
>> wNdpOutDivisor.
>>
>> We can choose to put the NDP at the end of the NTB if we find that this
>> fixes some problem, but doing so by default for every NCM and MBIM
>> device is a bit risky. If we accept that some devices are so buggy that
>> the NDP cannot be placed anywhere (as required by the spec), then we
>> have to assume that this goes both ways.  Which means that moving the
>> NDP to the end of the NTB might break some other device.  We just don't
>> know that since we haven't ever tried it.
>>
>> And your fix doesn't really move it to the end either.  It just places
>> the NDP after the first ethernet packet.  Which happens to be the end if
>> there is only one packet in the NTB. But if we aggregate more packets
>> into this NTB then the result will look like this:
>>
>> NTH
>> eth packet 1
>> NDP
>> eth packet 2
>> ..
>> eth packet N
>>
>> I'm not convinced this modem will handle that if it cannot handle the
>> NDP being before the first packet...  This needs to be tested. Try
>> increasing /sys/class/net/wwan0/cdc_ncm/tx_timer_usecs to force the
>> driver to aggregate packets and see if everything still works.
>> Preferably while looking at the resulting NTB to verify that it does
>> contain more than one ethernet packet.
>>
>> I realize I sound a bit negative now.  This is absolutely not my
>> intention. This is great work, providing some real progress wrt figuring
>> out what goes on here.  Thanks a lot!  I am sure we can sort out the
>> remaining issues, which are really minor compared to what you have found
>> so far.
>>
>>
>>
>> Bjørn
>>
>>
This email and any files transmitted with it are confidential material. They are intended solely for the use of the designated individual or entity to whom they are addressed. If the reader of this message is not the intended recipient, you are hereby notified that any dissemination, use, distribution or copying of this communication is strictly prohibited and may be unlawful.

If you have received this email in error please immediately notify the sender and delete or destroy any copy of this message
N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·¥Š{±ºÆâžØ^n‡r¡ö¦zË\x1aëh™¨è­Ú&¢îý»\x05ËÛÔØï¦v¬Îf\x1dp)¹¹br	šê+€Ê+zf£¢·hšˆ§~†­†Ûiÿûàz¹\x1e®w¥¢¸?™¨è­Ú&¢)ߢ^[f

^ permalink raw reply

* Re: [PATCH] x86: bpf_jit_comp: Remove inline from static function definitions
From: Joe Perches @ 2014-12-05  1:43 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Eric Dumazet, Quentin Lambert, David S. Miller, Alexey Kuznetsov,
	James Morris, Hideaki YOSHIFUJI, Patrick McHardy, Thomas Gleixner,
	Ingo Molnar, H. Peter Anvin, x86@kernel.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <CAADnVQ+5T3MDiC7RwVwdyu2PEhGy0VOrp7=LWeENrX_USXevnw@mail.gmail.com>

On Thu, 2014-12-04 at 17:21 -0800, Alexei Starovoitov wrote:
> Joe, please mention dependencies next time.

No real need here.
It's in the same thread.

^ permalink raw reply

* Re: [PATCH] x86: bpf_jit_comp: Remove inline from static function definitions
From: Alexei Starovoitov @ 2014-12-05  1:21 UTC (permalink / raw)
  To: Joe Perches
  Cc: Eric Dumazet, Quentin Lambert, David S. Miller, Alexey Kuznetsov,
	James Morris, Hideaki YOSHIFUJI, Patrick McHardy, Thomas Gleixner,
	Ingo Molnar, H. Peter Anvin, x86@kernel.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <1417741284.2721.26.camel@perches.com>

On Thu, Dec 4, 2014 at 5:01 PM, Joe Perches <joe@perches.com> wrote:
> Let the compiler decide instead.
>
> No change in object size x86-64 -O2 no profiling
>
> Signed-off-by: Joe Perches <joe@perches.com>
> Suggested-by: Eric Dumazet <eric.dumazet@gmail.com>

Acked-by: Alexei Starovoitov <ast@plumgrid.com>

Dave, this is on top of previous patch:
http://patchwork.ozlabs.org/patch/417960/

Joe, please mention dependencies next time.

^ permalink raw reply

* Re: [PATCH net-next 3/3] ip: Add support for IP_CHECKSUM cmsg
From: Eric Dumazet @ 2014-12-05  1:17 UTC (permalink / raw)
  To: Tom Herbert; +Cc: davem, netdev
In-Reply-To: <1417653868-14922-4-git-send-email-therbert@google.com>

On Wed, 2014-12-03 at 16:44 -0800, Tom Herbert wrote:

> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> index 221b53f..bba2e06 100644
> --- a/net/ipv4/udp.c
> +++ b/net/ipv4/udp.c
> @@ -1315,8 +1315,16 @@ try_again:
>  		memset(sin->sin_zero, 0, sizeof(sin->sin_zero));
>  		*addr_len = sizeof(*sin);
>  	}
> -	if (inet->cmsg_flags)
> +	if (inet->cmsg_flags) {
> +		/* Pull checksum past UDP header in case we are providing
> +		 * checksum in cmsg.
> +		 */
> +		if (inet->cmsg_flags & IP_CMSG_CHECKSUM)
> +			skb_postpull_rcsum(skb, skb->data,
> +					   sizeof(struct udphdr));
> +
>  		ip_cmsg_recv(msg, skb);
> +	}


Have you tried MSG_PEEK ?

You can not modify the skb, you need to do something else.

^ permalink raw reply

* Re: [PATCH net-next 1/2] tcp_cubic: add SNMP counters to track how effective is Hystart
From: Eric Dumazet @ 2014-12-05  1:10 UTC (permalink / raw)
  To: Florian Westphal
  Cc: David Miller, netdev, Nandita Dukkipati, Neal Cardwell,
	Yuchung Cheng, Sangtae Ha
In-Reply-To: <20141205001754.GG16959@breakpoint.cc>

On Fri, 2014-12-05 at 01:17 +0100, Florian Westphal wrote:

> Alternatively we could add INET_DIAG_CUBICINFO and export such info via ss
> tool.

Thats a complete different model.

When studying some param changes on servers handling million of flows,
SNMP hosts stats are giving the picture, without having to collect huge
ss info in the background and aggregating it.

^ permalink raw reply

* [PATCH] x86: bpf_jit_comp: Remove inline from static function definitions
From: Joe Perches @ 2014-12-05  1:01 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Alexei Starovoitov, Quentin Lambert, David S. Miller,
	Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI,
	Patrick McHardy, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	x86@kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <1417740382.22424.40.camel@edumazet-glaptop2.roam.corp.google.com>

Let the compiler decide instead.

No change in object size x86-64 -O2 no profiling

Signed-off-by: Joe Perches <joe@perches.com>
Suggested-by: Eric Dumazet <eric.dumazet@gmail.com>
---
On Thu, 2014-12-04 at 16:46 -0800, Eric Dumazet wrote:
> On Thu, 2014-12-04 at 15:31 -0800, Alexei Starovoitov wrote:
> 
> > well, it is a trivial function even from compiler point of view.
> > Dropping inline keyword doesn't help. gcc still inlines them.
> > Changing all 3 functions to _noinline_ doesn't help either.
> > So I think this patch is actually quite helpful to reduce code size.
> 
> Well, again this might depend on CONFIG_CC_OPTIMIZE_FOR_SIZE

-Os has a different size delta, but it's still
smaller using this new function.

> I guess people trying to get very small kernels are using this option.
> 
> My point was : If we care about code size, we should also remove these
> inline keywords at the same time, to increase SNR of netdev/lkml lists.

Because there's no object change here, inline removals
would probably be a good thing for this file.

 arch/x86/net/bpf_jit_comp.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index 09e2cea..626e013 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -24,7 +24,7 @@ extern u8 sk_load_byte_positive_offset[];
 extern u8 sk_load_word_negative_offset[], sk_load_half_negative_offset[];
 extern u8 sk_load_byte_negative_offset[];
 
-static inline u8 *emit_code(u8 *ptr, u32 bytes, unsigned int len)
+static u8 *emit_code(u8 *ptr, u32 bytes, unsigned int len)
 {
 	if (len == 1)
 		*ptr = bytes;
@@ -52,12 +52,12 @@ static inline u8 *emit_code(u8 *ptr, u32 bytes, unsigned int len)
 #define EMIT4_off32(b1, b2, b3, b4, off) \
 	do {EMIT4(b1, b2, b3, b4); EMIT(off, 4); } while (0)
 
-static inline bool is_imm8(int value)
+static bool is_imm8(int value)
 {
 	return value <= 127 && value >= -128;
 }
 
-static inline bool is_simm32(s64 value)
+static bool is_simm32(s64 value)
 {
 	return value == (s64) (s32) value;
 }
@@ -94,7 +94,7 @@ static int bpf_size_to_x86_bytes(int bpf_size)
 #define X86_JGE 0x7D
 #define X86_JG  0x7F
 
-static inline void bpf_flush_icache(void *start, void *end)
+static void bpf_flush_icache(void *start, void *end)
 {
 	mm_segment_t old_fs = get_fs();
 
@@ -133,7 +133,7 @@ static const int reg2hex[] = {
  * which need extra byte of encoding.
  * rax,rcx,...,rbp have simpler encoding
  */
-static inline bool is_ereg(u32 reg)
+static bool is_ereg(u32 reg)
 {
 	return (1 << reg) & (BIT(BPF_REG_5) |
 			     BIT(AUX_REG) |
@@ -143,14 +143,14 @@ static inline bool is_ereg(u32 reg)
 }
 
 /* add modifiers if 'reg' maps to x64 registers r8..r15 */
-static inline u8 add_1mod(u8 byte, u32 reg)
+static u8 add_1mod(u8 byte, u32 reg)
 {
 	if (is_ereg(reg))
 		byte |= 1;
 	return byte;
 }
 
-static inline u8 add_2mod(u8 byte, u32 r1, u32 r2)
+static u8 add_2mod(u8 byte, u32 r1, u32 r2)
 {
 	if (is_ereg(r1))
 		byte |= 1;
@@ -160,13 +160,13 @@ static inline u8 add_2mod(u8 byte, u32 r1, u32 r2)
 }
 
 /* encode 'dst_reg' register into x64 opcode 'byte' */
-static inline u8 add_1reg(u8 byte, u32 dst_reg)
+static u8 add_1reg(u8 byte, u32 dst_reg)
 {
 	return byte + reg2hex[dst_reg];
 }
 
 /* encode 'dst_reg' and 'src_reg' registers into x64 opcode 'byte' */
-static inline u8 add_2reg(u8 byte, u32 dst_reg, u32 src_reg)
+static u8 add_2reg(u8 byte, u32 dst_reg, u32 src_reg)
 {
 	return byte + reg2hex[dst_reg] + (reg2hex[src_reg] << 3);
 }

^ permalink raw reply related

* Re: [PATCH net-next 1/2] tcp_cubic: add SNMP counters to track how effective is Hystart
From: Neal Cardwell @ 2014-12-05  0:55 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David Miller, netdev, Nandita Dukkipati, Yuchung Cheng,
	Sangtae Ha
In-Reply-To: <1417738403.22424.28.camel@edumazet-glaptop2.roam.corp.google.com>

On Thu, Dec 4, 2014 at 7:13 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> When deploying FQ pacing, one thing we noticed is that CUBIC Hystart
> triggers too soon.
>
> Having SNMP counters to have an idea of how often the various Hystart
> methods trigger is useful prior to any modifications.
>
> This patch adds SNMP counters tracking, how many time "ack train" or
> "Delay" based Hystart triggers, and cumulative sum of cwnd at the time
> Hystart decided to end SS (Slow Start)
>
> myhost:~# nstat -a | grep Hystart
> TcpExtTCPHystartTrainDetect     9                  0.0
> TcpExtTCPHystartTrainCwnd       20650              0.0
> TcpExtTCPHystartDelayDetect     10                 0.0
> TcpExtTCPHystartDelayCwnd       360                0.0
>
> ->
>  Train detection was triggered 9 times, and average cwnd was
>  20650/9=2294,
>  Delay detection was triggered 10 times and average cwnd was 36
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Acked-by: Neal Cardwell <ncardwell@google.com>

Very nice. IMHO it's great to have these as SNMP counters, so we don't
need special tools to get aggregate stats for all connections.

neal

^ permalink raw reply

* Re: [PATCH net-next 2/2] tcp_cubic: refine Hystart delay threshold
From: Neal Cardwell @ 2014-12-05  0:48 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David Miller, netdev, Nandita Dukkipati, Yuchung Cheng,
	Sangtae Ha
In-Reply-To: <1417738429.22424.29.camel@edumazet-glaptop2.roam.corp.google.com>

On Thu, Dec 4, 2014 at 7:13 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> In commit 2b4636a5f8ca ("tcp_cubic: make the delay threshold of HyStart
> less sensitive"), HYSTART_DELAY_MIN was changed to 4 ms.
>
> The remaining problem is that using delay_min + (delay_min/16) as the
> threshold is too sensitive.
>
> 6.25 % of variation is too small for rtt above 60 ms, which are not
> uncommon.
>
> Lets use 12.5 % instead (delay_min + (delay_min/8))

Very nice, Eric!

Acked-by: Neal Cardwell <ncardwell@google.com>
Tested-by: Neal Cardwell <ncardwell@google.com>

neal

^ permalink raw reply

* Re: [PATCH] x86: bpf_jit_comp: Reduce is_ereg() code size
From: Eric Dumazet @ 2014-12-05  0:46 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Joe Perches, Quentin Lambert, David S. Miller, Alexey Kuznetsov,
	James Morris, Hideaki YOSHIFUJI, Patrick McHardy, Thomas Gleixner,
	Ingo Molnar, H. Peter Anvin, x86@kernel.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <CAADnVQ+3jiYOYReRLPUsFeG1p66ezo-TXBLX3BGQQh7V8QX0tQ@mail.gmail.com>

On Thu, 2014-12-04 at 15:31 -0800, Alexei Starovoitov wrote:

> well, it is a trivial function even from compiler point of view.
> Dropping inline keyword doesn't help. gcc still inlines them.
> Changing all 3 functions to _noinline_ doesn't help either.
> So I think this patch is actually quite helpful to reduce code size.

Well, again this might depend on CONFIG_CC_OPTIMIZE_FOR_SIZE

I guess people trying to get very small kernels are using this option.

My point was : If we care about code size, we should also remove these
inline keywords at the same time, to increase SNR of netdev/lkml lists.

^ permalink raw reply

* [PATCH iproute2] ip monitor: Fix issue when timestamp is printed w/o msg
From: Vadim Kochan @ 2014-12-05  0:18 UTC (permalink / raw)
  To: netdev; +Cc: Vadim Kochan

The issue was observed when IPv6 router broadcasted NDUSEROPT
messages which are not handled by monitor and caused printing
'Timestamps' w/o message because such kind of rtnl messages is not
handled by monitor.

As 'ip monitor' by default subscribes to the all mcast rtnl groups except
RTGRP_TC then all messages of these rtnl groups which are not handled by
monitor may cause such issues.

Fixed by subscribing by default to rtnl mcast groups which are
supported by 'ip monitor'.

Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
---
 ip/ipmonitor.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/ip/ipmonitor.c b/ip/ipmonitor.c
index 4cc75f4..4708e54 100644
--- a/ip/ipmonitor.c
+++ b/ip/ipmonitor.c
@@ -134,14 +134,6 @@ static int accept_msg(const struct sockaddr_nl *who,
 		fprintf(fp, "Timestamp: %s %lu us\n", tstr, usecs);
 		return 0;
 	}
-	if (n->nlmsg_type == RTM_NEWQDISC ||
-	    n->nlmsg_type == RTM_DELQDISC ||
-	    n->nlmsg_type == RTM_NEWTCLASS ||
-	    n->nlmsg_type == RTM_DELTCLASS ||
-	    n->nlmsg_type == RTM_NEWTFILTER ||
-	    n->nlmsg_type == RTM_DELTFILTER ||
-	    n->nlmsg_type == RTM_NEWNDUSEROPT)
-		return 0;
 	if (n->nlmsg_type != NLMSG_ERROR && n->nlmsg_type != NLMSG_NOOP &&
 	    n->nlmsg_type != NLMSG_DONE) {
 		fprintf(fp, "Unknown message: type=0x%08x(%d) flags=0x%08x(%d)"
@@ -155,7 +147,7 @@ static int accept_msg(const struct sockaddr_nl *who,
 int do_ipmonitor(int argc, char **argv)
 {
 	char *file = NULL;
-	unsigned groups = ~RTMGRP_TC;
+	unsigned groups = 0;
 	int llink=0;
 	int laddr=0;
 	int lroute=0;
@@ -165,6 +157,18 @@ int do_ipmonitor(int argc, char **argv)
 	int lnetconf=0;
 	int ifindex=0;
 
+	groups |= nl_mgrp(RTNLGRP_LINK);
+	groups |= nl_mgrp(RTNLGRP_IPV4_IFADDR);
+	groups |= nl_mgrp(RTNLGRP_IPV6_IFADDR);
+	groups |= nl_mgrp(RTNLGRP_IPV4_ROUTE);
+	groups |= nl_mgrp(RTNLGRP_IPV6_ROUTE);
+	groups |= nl_mgrp(RTNLGRP_IPV4_MROUTE);
+	groups |= nl_mgrp(RTNLGRP_IPV6_MROUTE);
+	groups |= nl_mgrp(RTNLGRP_IPV6_PREFIX);
+	groups |= nl_mgrp(RTNLGRP_NEIGH);
+	groups |= nl_mgrp(RTNLGRP_IPV4_NETCONF);
+	groups |= nl_mgrp(RTNLGRP_IPV6_NETCONF);
+
 	rtnl_close(&rth);
 
 	while (argc > 0) {
@@ -195,7 +199,6 @@ int do_ipmonitor(int argc, char **argv)
 			lnetconf = 1;
 			groups = 0;
 		} else if (strcmp(*argv, "all") == 0) {
-			groups = ~RTMGRP_TC;
 			prefix_banner=1;
 		} else if (matches(*argv, "help") == 0) {
 			usage();
-- 
2.1.3

^ permalink raw reply related

* Re: [PATCH net] gso: do GSO for local skb with size bigger than MTU
From: Du Fan @ 2014-12-05  0:25 UTC (permalink / raw)
  To: Jesse Gross
  Cc: Pravin Shelar, Thomas Graf, Michael S. Tsirkin, Du, Fan,
	Jason Wang, netdev@vger.kernel.org, davem@davemloft.net,
	fw@strlen.de, dev@openvswitch.org
In-Reply-To: <CAEP_g=9AH_RShKujVeUmty6BKPJZt=u_-h8NnUL1kX+tSYdBxQ@mail.gmail.com>


On 2014/12/5 7:23, Jesse Gross wrote:
> On Wed, Dec 3, 2014 at 11:48 PM, Du Fan <fengyuleidian0615@gmail.com> wrote:
>> 于 2014年12月04日 06:51, Jesse Gross 写道:
>>> My proposal would be something like this:
>>>    * For L2, reduce the VM MTU to the lowest common denominator on the
>>> segment.
>>>    * For L3, use path MTU discovery or fragment inner packet (i.e.
>>> normal routing behavior).
>>>    * As a last resort (such as if using an old version of virtio in the
>>> guest), fragment the tunnel packet.
>>
>> After some investigation on OpenvSwitch package, it seems before this
>> commit: 06021dcb "datapath: compat: Fix compilation 3.11" OpenvSwitch
>> package is doing GSO on its own.
>>
>> rpl_ip_local_out
>>    -> tnl_skb_gso_segment
>>        ^^^^^^^^^^^^^^^
>>           Perform GSO  in above function
>>      -> ip_local_out
>>            .
>>            .
>>          -> ip_finish_output
>>
>> Which means, when over-MTU-sized skb enter ip_local_out, it's not a gso
>> type skb, so the stack perform ip fragmentation, and send them out.
>> So, over-MTU-sized skb did travel through stack into outside.
>>
>> Why not dropping such OpenvSwitch level GSO operation after 3.10?
> The change in 3.11 was that the tunnel infrastructure used by OVS was
> upstreamed and shared by all implementations. It's not right to
> perform GSO in OVS itself as it prevents the logic from being used by
> other components. Breaking up the packet in OVS also eliminates some
> of the benefits of GSO by shortening the optimized path and prevents
> offloading to hardware.
Thanks for your explanation, I understand its background better now.

^ permalink raw reply

* Re: [PATCH net-next 1/2] tcp_cubic: add SNMP counters to track how effective is Hystart
From: Florian Westphal @ 2014-12-05  0:17 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David Miller, netdev, Nandita Dukkipati, Neal Cardwell,
	Yuchung Cheng, Sangtae Ha
In-Reply-To: <1417738403.22424.28.camel@edumazet-glaptop2.roam.corp.google.com>

Eric Dumazet <eric.dumazet@gmail.com> wrote:
> When deploying FQ pacing, one thing we noticed is that CUBIC Hystart
> triggers too soon.
> 
> Having SNMP counters to have an idea of how often the various Hystart
> methods trigger is useful prior to any modifications.
> 
> This patch adds SNMP counters tracking, how many time "ack train" or
> "Delay" based Hystart triggers, and cumulative sum of cwnd at the time
> Hystart decided to end SS (Slow Start)
> 
> myhost:~# nstat -a | grep Hystart
> TcpExtTCPHystartTrainDetect     9                  0.0
> TcpExtTCPHystartTrainCwnd       20650              0.0
> TcpExtTCPHystartDelayDetect     10                 0.0
> TcpExtTCPHystartDelayCwnd       360                0.0
> 
> ->
>  Train detection was triggered 9 times, and average cwnd was
>  20650/9=2294,
>  Delay detection was triggered 10 times and average cwnd was 36

Alternatively we could add INET_DIAG_CUBICINFO and export such info via ss
tool.

^ permalink raw reply

* [PATCH net-next 2/2] tcp_cubic: refine Hystart delay threshold
From: Eric Dumazet @ 2014-12-05  0:13 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, Nandita Dukkipati, Neal Cardwell, Yuchung Cheng,
	Sangtae Ha

From: Eric Dumazet <edumazet@google.com>

In commit 2b4636a5f8ca ("tcp_cubic: make the delay threshold of HyStart
less sensitive"), HYSTART_DELAY_MIN was changed to 4 ms.

The remaining problem is that using delay_min + (delay_min/16) as the
threshold is too sensitive.

6.25 % of variation is too small for rtt above 60 ms, which are not
uncommon.

Lets use 12.5 % instead (delay_min + (delay_min/8))

Tested:
 80 ms RTT between peers, FQ/pacing packet scheduler on sender.
 10 bulk transfers of 10 seconds :

nstat >/dev/null
for i in `seq 1 10`
 do
   netperf -H remote -- -k THROUGHPUT | grep THROUGHPUT
 done
nstat | grep Hystart

With the 6.25 % threshold :

THROUGHPUT=20.66
THROUGHPUT=249.38
THROUGHPUT=254.10
THROUGHPUT=14.94
THROUGHPUT=251.92
THROUGHPUT=237.73
THROUGHPUT=19.18
THROUGHPUT=252.89
THROUGHPUT=21.32
THROUGHPUT=15.58
TcpExtTCPHystartTrainDetect     2                  0.0
TcpExtTCPHystartTrainCwnd       4756               0.0
TcpExtTCPHystartDelayDetect     5                  0.0
TcpExtTCPHystartDelayCwnd       180                0.0

With the 12.5 % threshold 
THROUGHPUT=251.09
THROUGHPUT=247.46
THROUGHPUT=250.92
THROUGHPUT=248.91
THROUGHPUT=250.88
THROUGHPUT=249.84
THROUGHPUT=250.51
THROUGHPUT=254.15
THROUGHPUT=250.62
THROUGHPUT=250.89
TcpExtTCPHystartTrainDetect     1                  0.0
TcpExtTCPHystartTrainCwnd       3175               0.0

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/ipv4/tcp_cubic.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/tcp_cubic.c b/net/ipv4/tcp_cubic.c
index c1d07c7ed03d7d37fa28d1509093e686f78134d2..6b6002416a73950d493661ea1459870f49917efc 100644
--- a/net/ipv4/tcp_cubic.c
+++ b/net/ipv4/tcp_cubic.c
@@ -393,7 +393,7 @@ static void hystart_update(struct sock *sk, u32 delay)
 			ca->sample_cnt++;
 		} else {
 			if (ca->curr_rtt > ca->delay_min +
-			    HYSTART_DELAY_THRESH(ca->delay_min>>4)) {
+			    HYSTART_DELAY_THRESH(ca->delay_min >> 3)) {
 				ca->found |= HYSTART_DELAY;
 				NET_INC_STATS_BH(sock_net(sk),
 						 LINUX_MIB_TCPHYSTARTDELAYDETECT);

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox