Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH 19/51] DMA-API: media: dt3155v4l: replace dma_set_mask()+dma_set_coherent_mask() with new helper
From: Russell King - ARM Linux @ 2013-10-31 14:49 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Hans Verkuil, alsa-devel, linux-doc, linux-mmc, linux-fbdev,
	linux-nvme, linux-ide, devel, linux-samsung-soc, linux-scsi,
	e1000-devel, b43-dev, linux-media, devicetree, dri-devel,
	linux-tegra, linux-omap, linux-arm-kernel,
	Solarflare linux maintainers, netdev, linux-usb, linux-wireless,
	linux-crypto, Greg Kroah-Hartman, uclinux-dist-devel,
	linuxppc-dev
In-Reply-To: <20131031094640.205840a2@samsung.com>

On Thu, Oct 31, 2013 at 09:46:40AM -0200, Mauro Carvalho Chehab wrote:
> Hi Russell,
> 
> Em Mon, 30 Sep 2013 13:57:47 +0200
> Hans Verkuil <hverkuil@xs4all.nl> escreveu:
> 
> > On 09/19/2013 11:44 PM, Russell King wrote:
> > > Replace the following sequence:
> > > 
> > > 	dma_set_mask(dev, mask);
> > > 	dma_set_coherent_mask(dev, mask);
> > > 
> > > with a call to the new helper dma_set_mask_and_coherent().
> > > 
> > > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> > 
> > Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
> 
> Somehow, I lost your original post (I got unsubscribed on a few days 
> from all vger mailing lists at the end of september).
> 
> I suspect that you want to sent this via your tree, right?

Yes please.

> If so:
> 
> Acked-by: Mauro Carvalho Chehab <m.chehab@samsung.com>

Added, thanks.

> > > -	err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));
> > > -	if (err)
> > > -		return -ENODEV;
> > > -	err = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32));
> > > +	err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
> > >  	if (err)
> > >  		return -ENODEV;

One thing I've just noticed is that return should be "return err" not
"return -ENODEV" - are you okay for me to change that in this patch?

Thanks.

^ permalink raw reply

* [PATCH net-next 2/2] net: cdc_mbim: change the default to send ZLPs
From: Bjørn Mork @ 2013-10-31 14:56 UTC (permalink / raw)
  To: netdev; +Cc: Alexey Orishko, Greg Suarez, Bjørn Mork
In-Reply-To: <1383231371-8098-1-git-send-email-bjorn@mork.no>

A number of devices in the wild have turned out to require ZLPs.
Even if this is a spec violation, our priority is to make any
device work as good as possible. Devices needing ZLPs will fail
to receive any full sized frame we send. On the other hand,
devices which do not need the ZLP will still work if we send
them.

This gives us no other option than sending ZLPs by default.

This will prevent devices conforming to the spec from making the
optimizations which are possible without ZLPs.  Adding known
such devices to a whitelist, to avoid the possible negative
impact of the new spec violating default.

Cc: Greg Suarez <gsuarez@smithmicro.com>
Cc: Alexey Orishko <alexey.orishko@gmail.com>
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 drivers/net/usb/cdc_mbim.c |   28 +++++++++++++++++-----------
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/drivers/net/usb/cdc_mbim.c b/drivers/net/usb/cdc_mbim.c
index 6533a9f..253472a 100644
--- a/drivers/net/usb/cdc_mbim.c
+++ b/drivers/net/usb/cdc_mbim.c
@@ -429,9 +429,18 @@ static const struct driver_info cdc_mbim_info = {
 };
 
 /* MBIM and NCM devices should not need a ZLP after NTBs with
- * dwNtbOutMaxSize length. This driver_info is for the exceptional
- * devices requiring it anyway, allowing them to be supported without
- * forcing the performance penalty on all the sane devices.
+ * dwNtbOutMaxSize length. Nevertheless, a number of devices from
+ * different vendor IDs will fail unless we send ZLPs, forcing us
+ * to make this the default.
+ *
+ * This default may cause a performance penalty for spec conforming
+ * devices wanting to take advantage of optimizations possible without
+ * ZLPs.  A whitelist is added in an attempt to avoid this for devices
+ * known to conform to the MBIM specification.
+ *
+ * All known devices supporting NCM compatibility mode are also
+ * conforming to the NCM and MBIM specifications. For this reason, the
+ * NCM subclass entry is also in the ZLP whitelist.
  */
 static const struct driver_info cdc_mbim_info_zlp = {
 	.description = "CDC MBIM",
@@ -454,16 +463,13 @@ static const struct usb_device_id mbim_devs[] = {
 	{ USB_INTERFACE_INFO(USB_CLASS_COMM, USB_CDC_SUBCLASS_NCM, USB_CDC_PROTO_NONE),
 	  .driver_info = (unsigned long)&cdc_mbim_info,
 	},
-	/* Sierra Wireless MC7710 need ZLPs */
-	{ USB_DEVICE_AND_INTERFACE_INFO(0x1199, 0x68a2, USB_CLASS_COMM, USB_CDC_SUBCLASS_MBIM, USB_CDC_PROTO_NONE),
-	  .driver_info = (unsigned long)&cdc_mbim_info_zlp,
-	},
-	/* HP hs2434 Mobile Broadband Module needs ZLPs */
-	{ USB_DEVICE_AND_INTERFACE_INFO(0x3f0, 0x4b1d, USB_CLASS_COMM, USB_CDC_SUBCLASS_MBIM, USB_CDC_PROTO_NONE),
-	  .driver_info = (unsigned long)&cdc_mbim_info_zlp,
+	/* ZLP conformance whitelist: All Ericsson MBIM devices */
+	{ USB_VENDOR_AND_INTERFACE_INFO(0x0bdb, USB_CLASS_COMM, USB_CDC_SUBCLASS_MBIM, USB_CDC_PROTO_NONE),
+	  .driver_info = (unsigned long)&cdc_mbim_info,
 	},
+	/* default entry */
 	{ USB_INTERFACE_INFO(USB_CLASS_COMM, USB_CDC_SUBCLASS_MBIM, USB_CDC_PROTO_NONE),
-	  .driver_info = (unsigned long)&cdc_mbim_info,
+	  .driver_info = (unsigned long)&cdc_mbim_info_zlp,
 	},
 	{
 	},
-- 
1.7.10.4

^ permalink raw reply related

* Re: [RFC] ipv6: always join solicited-node address
From: Bjørn Mork @ 2013-10-31 15:00 UTC (permalink / raw)
  To: netdev
In-Reply-To: <20131014155610.GA20327@order.stressinduktion.org>

Hannes Frederic Sowa <hannes@stressinduktion.org> writes:

> On Mon, Oct 14, 2013 at 09:09:59AM +0200, Bjørn Mork wrote:
>> Yes, but that would also make the IP layer try to resolve IP to link
>> layer addressess both for IPv4 and IPv6, which just won't work. At least
>> not for IPv4, where there just is no way to transport an ARP to the
>> modem.  And I assume it may fail for IPv6 too on any sane device.
>
> I don't think that clearing the IFF_NOARP flag would kill connectivity
> for either IPv4 or IPv6.

It does, because the neigbours cannot be resolved.

> It may compromise security for IPv6 though
> (no idea how the telco network behind the modem looks like).
>
>> > Is this a specific bug of the modem you are using or are all devices
>> > powered by this driver like this?
>> 
>> Unfortunately I have no IPv6 enabled SIM myself, so I have no
>> information about other devices.  This report was based on user
>> feedback.
>> 
>> I assume the bug is specific to this firmware implementation, probably
>> extending to all similar devices from the same vendor.  But it could be
>> more common than that.  The fact that the bug is there indicates that
>> this works just fine in Windows.
>> 
>> Yes, I realize that I am in ugly-hack-to-workaround-firmware-issues land
>> again... But it sure would be nice to have some way for a driver to
>> indicate that L2 neighbour tables are meaningless, but that any incoming
>> requests should still be answered.
>
> L2 neighbour tables are resolved on demand and won't be queried for the
> link you are talking about (at least for IPv6, but I assume IPv4, too).
>
> A new flag should have clear semantics then:
>
> * split IFF_NOARP to IFF_NOARP and IFF_NONDISC
> * split IFF_NOARP to IFF_NOLLRESOLV_RESPONSE and IFF_NOLLRESOLV_MODIFY
>   (each one flag which is applicable for both IPv4 and IPv6)
>
> I tend to lean towards the last alternative but still wonder if this is
> just overhead for this one buggy device.

Thanks a lot for your review and thoughts on this.  I do agree 100% with
your last comment here, and ended up with an in-driver workaround
instead.

It's not beautiful, but at least it doesn't add a lot of core code for
this very special purpose.



Bjørn

^ permalink raw reply

* [PATCH net-next 0/2] cdc_mbim workarounds for firmware bugs
From: Bjørn Mork @ 2013-10-31 14:56 UTC (permalink / raw)
  To: netdev; +Cc: Alexey Orishko, Greg Suarez, Bjørn Mork

The first patch is an attempt to work around a bug which prevents
some devices from working with IPv6. I guess handling ND in a driver
can be considered controversial, but it was the best alternative I
came up with.

The second is the "final solution" to the well known ZLP bug, which
as we feared is present in a number of devices using different
vendor IDs. Enabling this is definitely controversial, but I do not
see any other possible solution.  The blacklist has become
unmanagable, and I am pretty sure it is very incomplete due to
under-reporting.  Devices known to work without ZLPs can be added
to the whitelist.

Bjørn Mork (2):
  net: cdc_mbim: handle IPv6 Neigbor Solicitations
  net: cdc_mbim: change the default to send ZLPs

 drivers/net/usb/cdc_mbim.c |   86 ++++++++++++++++++++++++++++++++++++++------
 1 file changed, 75 insertions(+), 11 deletions(-)

-- 
1.7.10.4

^ permalink raw reply

* [PATCH net-next 1/2] net: cdc_mbim: handle IPv6 Neigbor Solicitations
From: Bjørn Mork @ 2013-10-31 14:56 UTC (permalink / raw)
  To: netdev; +Cc: Alexey Orishko, Greg Suarez, Bjørn Mork
In-Reply-To: <1383231371-8098-1-git-send-email-bjorn@mork.no>

MBIM is a point-to-point protocol transporting raw IP packets
with no L2 headers. Only IPv4 and IPv6 are supported. ARP in
particular is not, which is quite logical given the lack of
L2 headers.

The driver still emulates an ethernet interface, dropping all
unsupported protocols, and avoiding neigbour resolving by
setting the IFF_NOARP flag.

The MBIM specification does not explicitly forbid IPv6 Neighbor
Discovery, and it seems the other OS support will respond to
Neighbor Solicitations on MBIM links. There are therefore
buggy devices out there, which despite the pointlessness, still
require Neighbor Discovery for IPv6 over MBIM.

This is incompatible with the IFF_NOARP flag which disables
both ARP and ND.  We cannot support ARP in any case, so we
have to keep that flag. This patch implements a workaround
for the buggy devices, letting the driver respond directly
to Neighbor Solicitations from the device.

This is not optimal, but will have minimal effect on any sane
device.

Cc: Greg Suarez <gsuarez@smithmicro.com>
Reported-and-tested-by: Thomas Schäfer <tschaefer@t-online.de>
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 drivers/net/usb/cdc_mbim.c |   58 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/drivers/net/usb/cdc_mbim.c b/drivers/net/usb/cdc_mbim.c
index 25ba7ec..6533a9f 100644
--- a/drivers/net/usb/cdc_mbim.c
+++ b/drivers/net/usb/cdc_mbim.c
@@ -21,6 +21,8 @@
 #include <linux/usb/usbnet.h>
 #include <linux/usb/cdc-wdm.h>
 #include <linux/usb/cdc_ncm.h>
+#include <net/ipv6.h>
+#include <net/addrconf.h>
 
 /* driver specific data - must match cdc_ncm usage */
 struct cdc_mbim_state {
@@ -184,6 +186,60 @@ error:
 	return NULL;
 }
 
+/* Some devices are known to send Neigbor Solicitation messages and
+ * require Neigbor Advertisement replies.  The IPv6 core will not
+ * respond since IFF_NOARP is set, so we must handle them ourselves.
+ */
+static void do_neigh_solicit(struct usbnet *dev, u8 *buf, u16 tci)
+{
+	struct ipv6hdr *iph = (void *)buf;
+	struct nd_msg *msg = (void *)(iph + 1);
+	struct net_device *netdev;
+	struct inet6_dev *in6_dev;
+	bool is_router;
+
+	/* we'll only respond to requests from unicast addresses to
+	 * our solicited node addresses.
+	 */
+	if (!ipv6_addr_is_solict_mult(&iph->daddr) ||
+	    !(ipv6_addr_type(&iph->saddr) & IPV6_ADDR_UNICAST))
+		return;
+
+	/* need to send the NA on the VLAN dev, if any */
+	if (tci)
+		netdev = __vlan_find_dev_deep(dev->net, htons(ETH_P_8021Q),
+					      tci);
+	else
+		netdev = dev->net;
+	if (!netdev)
+		return;
+
+	in6_dev = in6_dev_get(netdev);
+	if (!in6_dev)
+		return;
+	is_router = !!in6_dev->cnf.forwarding;
+	in6_dev_put(in6_dev);
+
+	/* ipv6_stub != NULL if in6_dev_get returned an inet6_dev */
+	ipv6_stub->ndisc_send_na(netdev, NULL, &iph->saddr, &msg->target,
+				 is_router /* router */,
+				 true /* solicited */,
+				 false /* override */,
+				 true /* inc_opt */);
+}
+
+static bool is_neigh_solicit(u8 *buf, size_t len)
+{
+	struct ipv6hdr *iph = (void *)buf;
+	struct nd_msg *msg = (void *)(iph + 1);
+
+	return (len >= sizeof(struct ipv6hdr) + sizeof(struct nd_msg) &&
+		iph->nexthdr == IPPROTO_ICMPV6 &&
+		msg->icmph.icmp6_code == 0 &&
+		msg->icmph.icmp6_type == NDISC_NEIGHBOUR_SOLICITATION);
+}
+
+
 static struct sk_buff *cdc_mbim_process_dgram(struct usbnet *dev, u8 *buf, size_t len, u16 tci)
 {
 	__be16 proto = htons(ETH_P_802_3);
@@ -198,6 +254,8 @@ static struct sk_buff *cdc_mbim_process_dgram(struct usbnet *dev, u8 *buf, size_
 			proto = htons(ETH_P_IP);
 			break;
 		case 0x60:
+			if (is_neigh_solicit(buf, len))
+				do_neigh_solicit(dev, buf, tci);
 			proto = htons(ETH_P_IPV6);
 			break;
 		default:
-- 
1.7.10.4

^ permalink raw reply related

* Re: [net-next PATCH] net: codel: Avoid undefined behavior from signed overflow
From: Eric Dumazet @ 2013-10-31 15:10 UTC (permalink / raw)
  To: Jesper Dangaard Brouer
  Cc: Jesper Dangaard Brouer, netdev, Paul E. McKenney, Dave Taht
In-Reply-To: <20131031151551.675ab908@redhat.com>

On Thu, 2013-10-31 at 15:15 +0100, Jesper Dangaard Brouer wrote:

> Okay, I'll cook up another patch, after work.
> 
> Adding all the typecheck() stuff, just bloats the code.
> 
> Would it be better/okay just to do?:
>  (s32)((u32)(a) - (u32)(b)) > 0)
> 
> 

What about using the existing codel types ?

diff --git a/include/net/codel.h b/include/net/codel.h
index 389cf62..89a7781 100644
--- a/include/net/codel.h
+++ b/include/net/codel.h
@@ -72,7 +72,12 @@ static inline codel_time_t codel_get_time(void)
 	return ns >> CODEL_SHIFT;
 }
 
-#define codel_time_after(a, b)		((s32)(a) - (s32)(b) > 0)
+static inline bool codel_time_after(codel_time_t a, codel_time_t b)
+{
+	codel_tdiff_t delta = a - b;
+
+	return delta >= 0;
+}
 #define codel_time_after_eq(a, b)	((s32)(a) - (s32)(b) >= 0)
 #define codel_time_before(a, b)		((s32)(a) - (s32)(b) < 0)
 #define codel_time_before_eq(a, b)	((s32)(a) - (s32)(b) <= 0)


You need of course something similar for all variants.

^ permalink raw reply related

* [PATCH net-next] tcp: enable sockets to use MSG_FASTOPEN by default
From: Yuchung Cheng @ 2013-10-31 16:19 UTC (permalink / raw)
  To: davem, edumazet; +Cc: netdev, ncardwell, sivasankar, Yuchung Cheng

Applications have started to use Fast Open (e.g., Chrome browser has
such an optional flag) and the feature has gone through several
generations of kernels since 3.7 with many real network tests. It's
time to enable this flag by default for applications to test more
conveniently and extensively.

Signed-off-by: Yuchung Cheng <ycheng@google.com>
Signed-off-by: Neal Cardwell <ncardwell@google.com>
---
 Documentation/networking/ip-sysctl.txt | 4 ++--
 net/ipv4/tcp_fastopen.c                | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index a46d785..6c00983 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -451,7 +451,7 @@ tcp_fastopen - INTEGER
 	connect() to perform a TCP handshake automatically.
 
 	The values (bitmap) are
-	1: Enables sending data in the opening SYN on the client.
+	1: Enables sending data in the opening SYN on the client w/ MSG_FASTOPEN.
 	2: Enables TCP Fast Open on the server side, i.e., allowing data in
 	   a SYN packet to be accepted and passed to the application before
 	   3-way hand shake finishes.
@@ -464,7 +464,7 @@ tcp_fastopen - INTEGER
 	   different ways of setting max_qlen without the TCP_FASTOPEN socket
 	   option.
 
-	Default: 0
+	Default: 1
 
 	Note that the client & server side Fast Open flags (1 and 2
 	respectively) must be also enabled before the rest of flags can take
diff --git a/net/ipv4/tcp_fastopen.c b/net/ipv4/tcp_fastopen.c
index 766032b..f195d93 100644
--- a/net/ipv4/tcp_fastopen.c
+++ b/net/ipv4/tcp_fastopen.c
@@ -8,7 +8,7 @@
 #include <net/inetpeer.h>
 #include <net/tcp.h>
 
-int sysctl_tcp_fastopen __read_mostly;
+int sysctl_tcp_fastopen __read_mostly = TFO_CLIENT_ENABLE;
 
 struct tcp_fastopen_context __rcu *tcp_fastopen_ctx;
 
-- 
1.8.4.1

^ permalink raw reply related

* Re: [PATCH v2 net-next] net: pkt_sched: PIE AQM scheme
From: Eric Dumazet @ 2013-10-31 16:52 UTC (permalink / raw)
  To: Vijay Subramanian; +Cc: netdev, davem, shemminger, Mythili Prabhu, Dave Taht
In-Reply-To: <1383167922-22495-1-git-send-email-subramanian.vijay@gmail.com>

On Wed, 2013-10-30 at 14:18 -0700, Vijay Subramanian wrote:

> +static void pie_destroy(struct Qdisc *sch)
> +{
> +	struct pie_sched_data *q = qdisc_priv(sch);
> +	del_timer_sync(&q->adapt_timer);
> +}

This doesn't look safe to me.

timer can rearm itself.

Take a look at commit 980c478ddbb720948967b028ddbb4179a025bc2c

^ permalink raw reply

* Re: [PATCH v2 net-next] net: pkt_sched: PIE AQM scheme
From: Eric Dumazet @ 2013-10-31 17:03 UTC (permalink / raw)
  To: Vijay Subramanian; +Cc: netdev, davem, shemminger, Mythili Prabhu, Dave Taht
In-Reply-To: <1383167922-22495-1-git-send-email-subramanian.vijay@gmail.com>

On Wed, 2013-10-30 at 14:18 -0700, Vijay Subramanian wrote:


+       if (tb[TCA_PIE_TUPDATE]) {
+               /* tupdate is in us */
+               u32 tupdate = nla_get_u32(tb[TCA_PIE_TUPDATE]);
+               /* convert to pschedtime */
+               q->params.tupdate = PSCHED_NS2TICKS((u64) tupdate * NSEC_PER_USEC);
+       }

...

> +static void pie_timer(unsigned long arg)
> +{
> +	struct Qdisc *sch = (struct Qdisc *)arg;
> +	struct pie_sched_data *q = qdisc_priv(sch);
> +	u32 tup;
> +	spinlock_t *root_lock = qdisc_lock(qdisc_root_sleeping(sch));
> +
> +	spin_lock(root_lock);
> +	calculate_probability(sch);
> +
> +	/* reset the timer to fire after 'tupdate'. tupdate is currently in
> +	 * psched_time; mod_timer expects time to be in jiffies so convert from
> +	 * pschedtime to jiffies
> +	 */
> +	tup = PSCHED_TICKS2NS(q->params.tupdate);
> +	tup = tup / NSEC_PER_MSEC;
> +	tup = (tup * HZ) / MSEC_PER_SEC;
> +
> +	mod_timer(&q->adapt_timer, jiffies + tup);
> +	spin_unlock(root_lock);
> +
> +}

Oh well, this makes no sense.

mod_timer() resolution is in jiffies, so please maintains params.tupdate
in usec, and use usecs_to_jiffies()

	mod_timer(&q->adapt_timer, jiffies +
				   usecs_to_jiffies(q->params.tupdate));

^ permalink raw reply

* [PATCH net-next] tcp: properly handle stretch acks in slow start
From: Yuchung Cheng @ 2013-10-31 18:07 UTC (permalink / raw)
  To: davem; +Cc: netdev, edumazet, ncardwell, vanj, Yuchung Cheng

Slow start now increases cwnd by 1 if an ACK acknowledges some packets,
regardless the number of packets. Consequently slow start performance
is highly dependent on the degree of the stretch ACKs caused by
receiver or network ACK compression mechanisms (e.g., delayed-ACK,
GRO, etc).  But slow start algorithm is to send twice the amount of
packets of packets left so it should process a stretch ACK of degree
N as if N ACKs of degree 1, then exits when cwnd exceeds ssthresh. A
follow up patch will use the remainder of the N (if greater than 1)
to adjust cwnd in the congestion avoidance phase.

In addition this patch retires the experimental limited slow start
(LSS) feature. LSS has multiple drawbacks but questionable benefit. The
fractional cwnd increase in LSS requires a loop in slow start even
though it's rarely used. Configuring such an increase step via a global
sysctl on different BDPS seems hard. Finally and most importantly the
slow start overshoot concern is now better covered by the Hybrid slow
start (hystart) enabled by default.

Signed-off-by: Yuchung Cheng <ycheng@google.com>
Signed-off-by: Neal Cardwell <ncardwell@google.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 Documentation/networking/ip-sysctl.txt | 11 --------
 include/net/tcp.h                      |  7 +++--
 net/ipv4/sysctl_net_ipv4.c             |  7 -----
 net/ipv4/tcp_bic.c                     |  5 ++--
 net/ipv4/tcp_cong.c                    | 47 ++++++++++++----------------------
 net/ipv4/tcp_cubic.c                   |  5 ++--
 net/ipv4/tcp_highspeed.c               |  4 +--
 net/ipv4/tcp_htcp.c                    |  4 +--
 net/ipv4/tcp_hybla.c                   |  5 ++--
 net/ipv4/tcp_illinois.c                |  5 ++--
 net/ipv4/tcp_input.c                   |  6 ++---
 net/ipv4/tcp_lp.c                      |  5 ++--
 net/ipv4/tcp_scalable.c                |  5 ++--
 net/ipv4/tcp_vegas.c                   | 11 ++++----
 net/ipv4/tcp_veno.c                    |  9 ++++---
 net/ipv4/tcp_yeah.c                    |  5 ++--
 16 files changed, 59 insertions(+), 82 deletions(-)

diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index 6c00983..8b8a057 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -267,17 +267,6 @@ tcp_max_orphans - INTEGER
 	more aggressively. Let me to remind again: each orphan eats
 	up to ~64K of unswappable memory.
 
-tcp_max_ssthresh - INTEGER
-	Limited Slow-Start for TCP with large congestion windows (cwnd) defined in
-	RFC3742. Limited slow-start is a mechanism to limit growth of the cwnd
-	on the region where cwnd is larger than tcp_max_ssthresh. TCP increases cwnd
-	by at most tcp_max_ssthresh segments, and by at least tcp_max_ssthresh/2
-	segments per RTT when the cwnd is above tcp_max_ssthresh.
-	If TCP connection increased cwnd to thousands (or tens of thousands) segments,
-	and thousands of packets were being dropped during slow-start, you can set
-	tcp_max_ssthresh to improve performance for new TCP connection.
-	Default: 0 (off)
-
 tcp_max_syn_backlog - INTEGER
 	Maximal number of remembered connection requests, which have not
 	received an acknowledgment from connecting client.
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 2d7b4bd..70e55d2 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -275,7 +275,6 @@ extern int sysctl_tcp_mtu_probing;
 extern int sysctl_tcp_base_mss;
 extern int sysctl_tcp_workaround_signed_windows;
 extern int sysctl_tcp_slow_start_after_idle;
-extern int sysctl_tcp_max_ssthresh;
 extern int sysctl_tcp_thin_linear_timeouts;
 extern int sysctl_tcp_thin_dupack;
 extern int sysctl_tcp_early_retrans;
@@ -797,7 +796,7 @@ struct tcp_congestion_ops {
 	/* lower bound for congestion window (optional) */
 	u32 (*min_cwnd)(const struct sock *sk);
 	/* do new cwnd calculation (required) */
-	void (*cong_avoid)(struct sock *sk, u32 ack, u32 in_flight);
+	void (*cong_avoid)(struct sock *sk, u32 ack, u32 acked, u32 in_flight);
 	/* call before changing ca_state (optional) */
 	void (*set_state)(struct sock *sk, u8 new_state);
 	/* call when cwnd event occurs (optional) */
@@ -824,12 +823,12 @@ void tcp_get_available_congestion_control(char *buf, size_t len);
 void tcp_get_allowed_congestion_control(char *buf, size_t len);
 int tcp_set_allowed_congestion_control(char *allowed);
 int tcp_set_congestion_control(struct sock *sk, const char *name);
-void tcp_slow_start(struct tcp_sock *tp);
+int tcp_slow_start(struct tcp_sock *tp, u32 acked);
 void tcp_cong_avoid_ai(struct tcp_sock *tp, u32 w);
 
 extern struct tcp_congestion_ops tcp_init_congestion_ops;
 u32 tcp_reno_ssthresh(struct sock *sk);
-void tcp_reno_cong_avoid(struct sock *sk, u32 ack, u32 in_flight);
+void tcp_reno_cong_avoid(struct sock *sk, u32 ack, u32 acked, u32 in_flight);
 u32 tcp_reno_min_cwnd(const struct sock *sk);
 extern struct tcp_congestion_ops tcp_reno;
 
diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index d5b1390..3d69ec8 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -701,13 +701,6 @@ static struct ctl_table ipv4_table[] = {
 		.proc_handler   = proc_allowed_congestion_control,
 	},
 	{
-		.procname	= "tcp_max_ssthresh",
-		.data		= &sysctl_tcp_max_ssthresh,
-		.maxlen		= sizeof(int),
-		.mode		= 0644,
-		.proc_handler	= proc_dointvec,
-	},
-	{
 		.procname       = "tcp_thin_linear_timeouts",
 		.data           = &sysctl_tcp_thin_linear_timeouts,
 		.maxlen         = sizeof(int),
diff --git a/net/ipv4/tcp_bic.c b/net/ipv4/tcp_bic.c
index f45e1c2..821846f 100644
--- a/net/ipv4/tcp_bic.c
+++ b/net/ipv4/tcp_bic.c
@@ -140,7 +140,8 @@ static inline void bictcp_update(struct bictcp *ca, u32 cwnd)
 		ca->cnt = 1;
 }
 
-static void bictcp_cong_avoid(struct sock *sk, u32 ack, u32 in_flight)
+static void bictcp_cong_avoid(struct sock *sk, u32 ack, u32 acked,
+			      u32 in_flight)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
 	struct bictcp *ca = inet_csk_ca(sk);
@@ -149,7 +150,7 @@ static void bictcp_cong_avoid(struct sock *sk, u32 ack, u32 in_flight)
 		return;
 
 	if (tp->snd_cwnd <= tp->snd_ssthresh)
-		tcp_slow_start(tp);
+		tcp_slow_start(tp, acked);
 	else {
 		bictcp_update(ca, tp->snd_cwnd);
 		tcp_cong_avoid_ai(tp, ca->cnt);
diff --git a/net/ipv4/tcp_cong.c b/net/ipv4/tcp_cong.c
index 019c238..ad37bf1 100644
--- a/net/ipv4/tcp_cong.c
+++ b/net/ipv4/tcp_cong.c
@@ -15,8 +15,6 @@
 #include <linux/gfp.h>
 #include <net/tcp.h>
 
-int sysctl_tcp_max_ssthresh = 0;
-
 static DEFINE_SPINLOCK(tcp_cong_list_lock);
 static LIST_HEAD(tcp_cong_list);
 
@@ -299,35 +297,24 @@ bool tcp_is_cwnd_limited(const struct sock *sk, u32 in_flight)
 }
 EXPORT_SYMBOL_GPL(tcp_is_cwnd_limited);
 
-/*
- * Slow start is used when congestion window is less than slow start
- * threshold. This version implements the basic RFC2581 version
- * and optionally supports:
- * 	RFC3742 Limited Slow Start  	  - growth limited to max_ssthresh
- *	RFC3465 Appropriate Byte Counting - growth limited by bytes acknowledged
+/* Slow start is used when congestion window is no greater than the slow start
+ * threshold. We base on RFC2581 and also handle stretch ACKs properly.
+ * We do not implement RFC3465 Appropriate Byte Counting (ABC) per se but
+ * something better;) a packet is only considered (s)acked in its entirety to
+ * defend the ACK attacks described in the RFC. Slow start processes a stretch
+ * ACK of degree N as if N acks of degree 1 are received back to back except
+ * ABC caps N to 2. Slow start exits when cwnd grows over ssthresh and
+ * returns the leftover acks to adjust cwnd in congestion avoidance mode.
  */
-void tcp_slow_start(struct tcp_sock *tp)
+int tcp_slow_start(struct tcp_sock *tp, u32 acked)
 {
-	int cnt; /* increase in packets */
-	unsigned int delta = 0;
-	u32 snd_cwnd = tp->snd_cwnd;
-
-	if (unlikely(!snd_cwnd)) {
-		pr_err_once("snd_cwnd is nul, please report this bug.\n");
-		snd_cwnd = 1U;
-	}
+	u32 cwnd = tp->snd_cwnd + acked;
 
-	if (sysctl_tcp_max_ssthresh > 0 && tp->snd_cwnd > sysctl_tcp_max_ssthresh)
-		cnt = sysctl_tcp_max_ssthresh >> 1;	/* limited slow start */
-	else
-		cnt = snd_cwnd;				/* exponential increase */
-
-	tp->snd_cwnd_cnt += cnt;
-	while (tp->snd_cwnd_cnt >= snd_cwnd) {
-		tp->snd_cwnd_cnt -= snd_cwnd;
-		delta++;
-	}
-	tp->snd_cwnd = min(snd_cwnd + delta, tp->snd_cwnd_clamp);
+	if (cwnd > tp->snd_ssthresh)
+		cwnd = tp->snd_ssthresh + 1;
+	acked -= cwnd - tp->snd_cwnd;
+	tp->snd_cwnd = min(cwnd, tp->snd_cwnd_clamp);
+	return acked;
 }
 EXPORT_SYMBOL_GPL(tcp_slow_start);
 
@@ -351,7 +338,7 @@ EXPORT_SYMBOL_GPL(tcp_cong_avoid_ai);
 /* This is Jacobson's slow start and congestion avoidance.
  * SIGCOMM '88, p. 328.
  */
-void tcp_reno_cong_avoid(struct sock *sk, u32 ack, u32 in_flight)
+void tcp_reno_cong_avoid(struct sock *sk, u32 ack, u32 acked, u32 in_flight)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
 
@@ -360,7 +347,7 @@ void tcp_reno_cong_avoid(struct sock *sk, u32 ack, u32 in_flight)
 
 	/* In "safe" area, increase. */
 	if (tp->snd_cwnd <= tp->snd_ssthresh)
-		tcp_slow_start(tp);
+		tcp_slow_start(tp, acked);
 	/* In dangerous area, increase slowly. */
 	else
 		tcp_cong_avoid_ai(tp, tp->snd_cwnd);
diff --git a/net/ipv4/tcp_cubic.c b/net/ipv4/tcp_cubic.c
index b6ae92a..828e4c3 100644
--- a/net/ipv4/tcp_cubic.c
+++ b/net/ipv4/tcp_cubic.c
@@ -304,7 +304,8 @@ static inline void bictcp_update(struct bictcp *ca, u32 cwnd)
 		ca->cnt = 1;
 }
 
-static void bictcp_cong_avoid(struct sock *sk, u32 ack, u32 in_flight)
+static void bictcp_cong_avoid(struct sock *sk, u32 ack, u32 acked,
+			      u32 in_flight)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
 	struct bictcp *ca = inet_csk_ca(sk);
@@ -315,7 +316,7 @@ static void bictcp_cong_avoid(struct sock *sk, u32 ack, u32 in_flight)
 	if (tp->snd_cwnd <= tp->snd_ssthresh) {
 		if (hystart && after(ack, ca->end_seq))
 			bictcp_hystart_reset(sk);
-		tcp_slow_start(tp);
+		tcp_slow_start(tp, acked);
 	} else {
 		bictcp_update(ca, tp->snd_cwnd);
 		tcp_cong_avoid_ai(tp, ca->cnt);
diff --git a/net/ipv4/tcp_highspeed.c b/net/ipv4/tcp_highspeed.c
index 30f27f6..8ed9305 100644
--- a/net/ipv4/tcp_highspeed.c
+++ b/net/ipv4/tcp_highspeed.c
@@ -109,7 +109,7 @@ static void hstcp_init(struct sock *sk)
 	tp->snd_cwnd_clamp = min_t(u32, tp->snd_cwnd_clamp, 0xffffffff/128);
 }
 
-static void hstcp_cong_avoid(struct sock *sk, u32 adk, u32 in_flight)
+static void hstcp_cong_avoid(struct sock *sk, u32 ack, u32 acked, u32 in_flight)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
 	struct hstcp *ca = inet_csk_ca(sk);
@@ -118,7 +118,7 @@ static void hstcp_cong_avoid(struct sock *sk, u32 adk, u32 in_flight)
 		return;
 
 	if (tp->snd_cwnd <= tp->snd_ssthresh)
-		tcp_slow_start(tp);
+		tcp_slow_start(tp, acked);
 	else {
 		/* Update AIMD parameters.
 		 *
diff --git a/net/ipv4/tcp_htcp.c b/net/ipv4/tcp_htcp.c
index c1a8175..4a194ac 100644
--- a/net/ipv4/tcp_htcp.c
+++ b/net/ipv4/tcp_htcp.c
@@ -227,7 +227,7 @@ static u32 htcp_recalc_ssthresh(struct sock *sk)
 	return max((tp->snd_cwnd * ca->beta) >> 7, 2U);
 }
 
-static void htcp_cong_avoid(struct sock *sk, u32 ack, u32 in_flight)
+static void htcp_cong_avoid(struct sock *sk, u32 ack, u32 acked, u32 in_flight)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
 	struct htcp *ca = inet_csk_ca(sk);
@@ -236,7 +236,7 @@ static void htcp_cong_avoid(struct sock *sk, u32 ack, u32 in_flight)
 		return;
 
 	if (tp->snd_cwnd <= tp->snd_ssthresh)
-		tcp_slow_start(tp);
+		tcp_slow_start(tp, acked);
 	else {
 		/* In dangerous area, increase slowly.
 		 * In theory this is tp->snd_cwnd += alpha / tp->snd_cwnd
diff --git a/net/ipv4/tcp_hybla.c b/net/ipv4/tcp_hybla.c
index 57bdd17..478fe82 100644
--- a/net/ipv4/tcp_hybla.c
+++ b/net/ipv4/tcp_hybla.c
@@ -85,7 +85,8 @@ static inline u32 hybla_fraction(u32 odds)
  *     o Give cwnd a new value based on the model proposed
  *     o remember increments <1
  */
-static void hybla_cong_avoid(struct sock *sk, u32 ack, u32 in_flight)
+static void hybla_cong_avoid(struct sock *sk, u32 ack, u32 acked,
+			     u32 in_flight)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
 	struct hybla *ca = inet_csk_ca(sk);
@@ -102,7 +103,7 @@ static void hybla_cong_avoid(struct sock *sk, u32 ack, u32 in_flight)
 		return;
 
 	if (!ca->hybla_en) {
-		tcp_reno_cong_avoid(sk, ack, in_flight);
+		tcp_reno_cong_avoid(sk, ack, acked, in_flight);
 		return;
 	}
 
diff --git a/net/ipv4/tcp_illinois.c b/net/ipv4/tcp_illinois.c
index 834857f..8a52099 100644
--- a/net/ipv4/tcp_illinois.c
+++ b/net/ipv4/tcp_illinois.c
@@ -256,7 +256,8 @@ static void tcp_illinois_state(struct sock *sk, u8 new_state)
 /*
  * Increase window in response to successful acknowledgment.
  */
-static void tcp_illinois_cong_avoid(struct sock *sk, u32 ack, u32 in_flight)
+static void tcp_illinois_cong_avoid(struct sock *sk, u32 ack, u32 acked,
+				    u32 in_flight)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
 	struct illinois *ca = inet_csk_ca(sk);
@@ -270,7 +271,7 @@ static void tcp_illinois_cong_avoid(struct sock *sk, u32 ack, u32 in_flight)
 
 	/* In slow start */
 	if (tp->snd_cwnd <= tp->snd_ssthresh)
-		tcp_slow_start(tp);
+		tcp_slow_start(tp, acked);
 
 	else {
 		u32 delta;
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index b935397..ad8d869 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -2928,10 +2928,10 @@ static void tcp_synack_rtt_meas(struct sock *sk, struct request_sock *req)
 	tcp_ack_update_rtt(sk, FLAG_SYN_ACKED, seq_rtt, -1);
 }
 
-static void tcp_cong_avoid(struct sock *sk, u32 ack, u32 in_flight)
+static void tcp_cong_avoid(struct sock *sk, u32 ack, u32 acked, u32 in_flight)
 {
 	const struct inet_connection_sock *icsk = inet_csk(sk);
-	icsk->icsk_ca_ops->cong_avoid(sk, ack, in_flight);
+	icsk->icsk_ca_ops->cong_avoid(sk, ack, acked, in_flight);
 	tcp_sk(sk)->snd_cwnd_stamp = tcp_time_stamp;
 }
 
@@ -3441,7 +3441,7 @@ static int tcp_ack(struct sock *sk, const struct sk_buff *skb, int flag)
 
 	/* Advance cwnd if state allows */
 	if (tcp_may_raise_cwnd(sk, flag))
-		tcp_cong_avoid(sk, ack, prior_in_flight);
+		tcp_cong_avoid(sk, ack, acked, prior_in_flight);
 
 	if (tcp_ack_is_dubious(sk, flag)) {
 		is_dupack = !(flag & (FLAG_SND_UNA_ADVANCED | FLAG_NOT_DUP));
diff --git a/net/ipv4/tcp_lp.c b/net/ipv4/tcp_lp.c
index 72f7218..991d62a 100644
--- a/net/ipv4/tcp_lp.c
+++ b/net/ipv4/tcp_lp.c
@@ -115,12 +115,13 @@ static void tcp_lp_init(struct sock *sk)
  * Will only call newReno CA when away from inference.
  * From TCP-LP's paper, this will be handled in additive increasement.
  */
-static void tcp_lp_cong_avoid(struct sock *sk, u32 ack, u32 in_flight)
+static void tcp_lp_cong_avoid(struct sock *sk, u32 ack, u32 acked,
+			      u32 in_flight)
 {
 	struct lp *lp = inet_csk_ca(sk);
 
 	if (!(lp->flag & LP_WITHIN_INF))
-		tcp_reno_cong_avoid(sk, ack, in_flight);
+		tcp_reno_cong_avoid(sk, ack, acked, in_flight);
 }
 
 /**
diff --git a/net/ipv4/tcp_scalable.c b/net/ipv4/tcp_scalable.c
index 8ce55b8..19ea6c2 100644
--- a/net/ipv4/tcp_scalable.c
+++ b/net/ipv4/tcp_scalable.c
@@ -15,7 +15,8 @@
 #define TCP_SCALABLE_AI_CNT	50U
 #define TCP_SCALABLE_MD_SCALE	3
 
-static void tcp_scalable_cong_avoid(struct sock *sk, u32 ack, u32 in_flight)
+static void tcp_scalable_cong_avoid(struct sock *sk, u32 ack, u32 acked,
+				    u32 in_flight)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
 
@@ -23,7 +24,7 @@ static void tcp_scalable_cong_avoid(struct sock *sk, u32 ack, u32 in_flight)
 		return;
 
 	if (tp->snd_cwnd <= tp->snd_ssthresh)
-		tcp_slow_start(tp);
+		tcp_slow_start(tp, acked);
 	else
 		tcp_cong_avoid_ai(tp, min(tp->snd_cwnd, TCP_SCALABLE_AI_CNT));
 }
diff --git a/net/ipv4/tcp_vegas.c b/net/ipv4/tcp_vegas.c
index 80fa2bf..06cae62 100644
--- a/net/ipv4/tcp_vegas.c
+++ b/net/ipv4/tcp_vegas.c
@@ -163,13 +163,14 @@ static inline u32 tcp_vegas_ssthresh(struct tcp_sock *tp)
 	return  min(tp->snd_ssthresh, tp->snd_cwnd-1);
 }
 
-static void tcp_vegas_cong_avoid(struct sock *sk, u32 ack, u32 in_flight)
+static void tcp_vegas_cong_avoid(struct sock *sk, u32 ack, u32 acked,
+				 u32 in_flight)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
 	struct vegas *vegas = inet_csk_ca(sk);
 
 	if (!vegas->doing_vegas_now) {
-		tcp_reno_cong_avoid(sk, ack, in_flight);
+		tcp_reno_cong_avoid(sk, ack, acked, in_flight);
 		return;
 	}
 
@@ -194,7 +195,7 @@ static void tcp_vegas_cong_avoid(struct sock *sk, u32 ack, u32 in_flight)
 			/* We don't have enough RTT samples to do the Vegas
 			 * calculation, so we'll behave like Reno.
 			 */
-			tcp_reno_cong_avoid(sk, ack, in_flight);
+			tcp_reno_cong_avoid(sk, ack, acked, in_flight);
 		} else {
 			u32 rtt, diff;
 			u64 target_cwnd;
@@ -243,7 +244,7 @@ static void tcp_vegas_cong_avoid(struct sock *sk, u32 ack, u32 in_flight)
 
 			} else if (tp->snd_cwnd <= tp->snd_ssthresh) {
 				/* Slow start.  */
-				tcp_slow_start(tp);
+				tcp_slow_start(tp, acked);
 			} else {
 				/* Congestion avoidance. */
 
@@ -283,7 +284,7 @@ static void tcp_vegas_cong_avoid(struct sock *sk, u32 ack, u32 in_flight)
 	}
 	/* Use normal slow start */
 	else if (tp->snd_cwnd <= tp->snd_ssthresh)
-		tcp_slow_start(tp);
+		tcp_slow_start(tp, acked);
 
 }
 
diff --git a/net/ipv4/tcp_veno.c b/net/ipv4/tcp_veno.c
index ac43cd7..326475a 100644
--- a/net/ipv4/tcp_veno.c
+++ b/net/ipv4/tcp_veno.c
@@ -114,13 +114,14 @@ static void tcp_veno_cwnd_event(struct sock *sk, enum tcp_ca_event event)
 		tcp_veno_init(sk);
 }
 
-static void tcp_veno_cong_avoid(struct sock *sk, u32 ack, u32 in_flight)
+static void tcp_veno_cong_avoid(struct sock *sk, u32 ack, u32 acked,
+				u32 in_flight)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
 	struct veno *veno = inet_csk_ca(sk);
 
 	if (!veno->doing_veno_now) {
-		tcp_reno_cong_avoid(sk, ack, in_flight);
+		tcp_reno_cong_avoid(sk, ack, acked, in_flight);
 		return;
 	}
 
@@ -133,7 +134,7 @@ static void tcp_veno_cong_avoid(struct sock *sk, u32 ack, u32 in_flight)
 		/* We don't have enough rtt samples to do the Veno
 		 * calculation, so we'll behave like Reno.
 		 */
-		tcp_reno_cong_avoid(sk, ack, in_flight);
+		tcp_reno_cong_avoid(sk, ack, acked, in_flight);
 	} else {
 		u64 target_cwnd;
 		u32 rtt;
@@ -152,7 +153,7 @@ static void tcp_veno_cong_avoid(struct sock *sk, u32 ack, u32 in_flight)
 
 		if (tp->snd_cwnd <= tp->snd_ssthresh) {
 			/* Slow start.  */
-			tcp_slow_start(tp);
+			tcp_slow_start(tp, acked);
 		} else {
 			/* Congestion avoidance. */
 			if (veno->diff < beta) {
diff --git a/net/ipv4/tcp_yeah.c b/net/ipv4/tcp_yeah.c
index 05c3b6f..a347a07 100644
--- a/net/ipv4/tcp_yeah.c
+++ b/net/ipv4/tcp_yeah.c
@@ -69,7 +69,8 @@ static void tcp_yeah_pkts_acked(struct sock *sk, u32 pkts_acked, s32 rtt_us)
 	tcp_vegas_pkts_acked(sk, pkts_acked, rtt_us);
 }
 
-static void tcp_yeah_cong_avoid(struct sock *sk, u32 ack, u32 in_flight)
+static void tcp_yeah_cong_avoid(struct sock *sk, u32 ack, u32 acked,
+				u32 in_flight)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
 	struct yeah *yeah = inet_csk_ca(sk);
@@ -78,7 +79,7 @@ static void tcp_yeah_cong_avoid(struct sock *sk, u32 ack, u32 in_flight)
 		return;
 
 	if (tp->snd_cwnd <= tp->snd_ssthresh)
-		tcp_slow_start(tp);
+		tcp_slow_start(tp, acked);
 
 	else if (!yeah->doing_reno_now) {
 		/* Scalable */
-- 
1.8.4.1

^ permalink raw reply related

* Re: [PATCH] x86: Run checksumming in parallel accross multiple alu's
From: Neil Horman @ 2013-10-31 18:30 UTC (permalink / raw)
  To: Doug Ledford
  Cc: Ingo Molnar, Eric Dumazet, linux-kernel, netdev, David Laight
In-Reply-To: <52710B09.6090302@redhat.com>

On Wed, Oct 30, 2013 at 09:35:05AM -0400, Doug Ledford wrote:
> On 10/30/2013 07:02 AM, Neil Horman wrote:
> 
> >That does makes sense, but it then begs the question, whats the advantage of
> >having multiple alu's at all?
> 
> There's lots of ALU operations that don't operate on the flags or
> other entities that can be run in parallel.
> 
> >If they're just going to serialize on the
> >updating of the condition register, there doesn't seem to be much advantage in
> >having multiple alu's at all, especially if a common use case (parallelizing an
> >operation on a large linear dataset) resulted in lower performance.
> >
> >/me wonders if rearranging the instructions into this order:
> >adcq 0*8(src), res1
> >adcq 1*8(src), res2
> >adcq 2*8(src), res1
> >
> >would prevent pipeline stalls.  That would be interesting data, and (I think)
> >support your theory, Doug.  I'll give that a try
> 
> Just to avoid spending too much time on various combinations, here
> are the methods I've tried:
> 
> Original code
> 2 chains doing interleaved memory accesses
> 2 chains doing serial memory accesses (as above)
> 4 chains doing serial memory accesses
> 4 chains using 32bit values in 64bit registers so you can always use
> add instead of adc and never need the carry flag
> 
> And I've done all of the above with simple prefetch and smart prefetch.
> 
> 


So, above and beyond this I spent yesterday trying this pattern, something Doug
and I discussed together offline:
 
asm("prefetch 5*64(%[src])\n\t"
    "addq 0*8(%[src]),%[res1]\n\t"
    "jo 2f\n\t"
    "incq %[cry]\n\t"
    "2:addq 1*8(%[src]),%[res2]\n\t"
    "jc 3f\n\t"
    "incq %[cry]\n\t"
    "3:addq 2*8(%[src]),%[res1]\n\t"
    ...

The hope being that by using the add instead instead of the adc instruction, and
alternatively testing the overflow and carry flags, I could break the
serialization on the flags register between subeuqent adds and start doing
things in parallel (creating a poor mans adcx/adox instruction in effect).  It
functions, but unfortunately the performance lost to the completely broken
branch prediction that this inflicts makes it a non starter:


Base Performance:
 Performance counter stats for './test.sh' (20 runs):

        48,143,372 L1-dcache-load-misses                                         ( +-  0.03% ) [74.99%]
                 0 L1-dcache-prefetches                                         [75.00%]
    13,913,339,911 cycles                    #    0.000 GHz                      ( +-  0.06% ) [75.01%]
        28,878,999 branch-misses                                                 ( +-  0.05% ) [75.00%]

       5.367618727 seconds time elapsed                                          ( +-  0.06% )


Prefetch and simluated adcx/adox from above:
 Performance counter stats for './test.sh' (20 runs):

        35,704,331 L1-dcache-load-misses                                         ( +-  0.07% ) [75.00%]
                 0 L1-dcache-prefetches                                         [75.00%]
    19,751,409,264 cycles                    #    0.000 GHz                      ( +-  0.59% ) [75.00%]
        34,850,056 branch-misses                                                 ( +-  1.29% ) [75.00%]

       7.768602160 seconds time elapsed                                          ( +-  1.38% )


With the above instruction changes the prefetching lowers our dcache miss rate
significantly, but greatly raises our branch miss rate, and absolutely kills our
cycle count and run time.

At this point I feel like this is dead in the water.  I apologize for wasting
everyones time.  The best thing to do here would seem to be:

1) Add in some prefetching (from what I've seen a simple prefetch is as
performant as smart prefetching), so we may as well do it exactly as
csum_copy_from_user does it, and save ourselves the extra while loop.

2) Revisit this when the AVX extensions, or the adcx/adox instructions are
available and we can really preform parallel alu ops here.

Does that sound reasonable?
Neil

^ permalink raw reply

* Good Day
From: BANK, OF, khairiya.masoud @ 2013-10-30 17:09 UTC (permalink / raw)
  To: Recipients

Good Day,
 
My name is Mr. Lee from Lars Jayden Limited, Thailand. i am interested in
purchasing your product, can you send me your latest product photo and
catalog. also send me your minimum order quantity and price list. hope to
hear from you soon.
 
Regards.
Purchase manager
Mr. Lee.
Address: Office No-57
cambride colony. Thailand

^ permalink raw reply

* Re: [gpio:for-next 67/67] pch_gbe_main.c:undefined reference to `devm_gpio_request_one'
From: David Cohen @ 2013-10-31 19:12 UTC (permalink / raw)
  To: Linus Walleij, Darren Hart
  Cc: David S. Miller, netdev@vger.kernel.org, Fengguang Wu,
	Alexandre Courbot, linux-gpio@vger.kernel.org
In-Reply-To: <CACRpkdZM8BBR6NAUTuSpEErZe2541-CvuwFSe13FDfxOBnnygA@mail.gmail.com>

On 10/30/2013 11:21 AM, Linus Walleij wrote:
> On Sat, Oct 26, 2013 at 10:31 PM, David Cohen
> <david.a.cohen@linux.intel.com> wrote:
> 
>> Maybe if GPIOLIB has the static inline stubs returning -ENODEV we could
>> use a patch similar to the one attached here.
> 
> I've added stubs returning -EINVAL just like all siblings there, if you
> like to have this changed, send a patch changing it to -ENODEV
> and this patch on top as 2/2 please.

Thanks.

Darren, do you have any objections WRT failing pch_gbe on Minnowboard
due to lack of GPIO?

BR, David

^ permalink raw reply

* Re: [Xen-devel] [PATCH net-next RFC 1/5] xen-netback: Introduce TX grant map definitions
From: Zoltan Kiss @ 2013-10-31 19:22 UTC (permalink / raw)
  To: Jan Beulich
  Cc: ian.campbell, jonathan.davies, wei.liu2, xen-devel, linux-kernel,
	netdev
In-Reply-To: <5270DF3602000078000FDFBE@nat28.tlf.novell.com>

On 30/10/13 09:28, Jan Beulich wrote:
>>>> On 30.10.13 at 01:50, Zoltan Kiss <zoltan.kiss@citrix.com> wrote:
>> @@ -119,13 +126,22 @@ struct xenvif {
>>   	pending_ring_idx_t pending_cons;
>>   	u16 pending_ring[MAX_PENDING_REQS];
>>   	struct pending_tx_info pending_tx_info[MAX_PENDING_REQS];
>> +	grant_handle_t grant_tx_handle[MAX_PENDING_REQS];
>>
>>   	/* Coalescing tx requests before copying makes number of grant
>>   	 * copy ops greater or equal to number of slots required. In
>>   	 * worst case a tx request consumes 2 gnttab_copy.
>>   	 */
>>   	struct gnttab_copy tx_copy_ops[2*MAX_PENDING_REQS];
>> +	struct gnttab_unmap_grant_ref tx_unmap_ops[MAX_PENDING_REQS];
>> +	struct gnttab_map_grant_ref tx_map_ops[MAX_PENDING_REQS];
>> +	/* passed to gnttab_[un]map_refs with pages under (un)mapping */
>> +	struct page *pages_to_gnt[MAX_PENDING_REQS];
>
> I think you will want to try to limit the structure size here by putting
> things into unions that can't be used at the same time: Without
> having looked at the later patches yet, it seems quite unlikely that
> map and unmap can be used simultaneously. And the total of copy
> and map can't also be used at the same time, as for each pending
> request you would use either up to two copy slots or a single map
> slot. I didn't look for further opportunities of sharing space.

Indeed, map and unmap can't be done at the same time, so it's safe to 
put them into union. But I'm afraid grant_tx_handle and pages_to_gnt 
can't share space with other members.
tx_copy_ops is a different topic, let's discuss that in it's own thread ...

Thanks,

Zoli

^ permalink raw reply

* [PATCH 1/2] can: c_can: Fix RX message handling, handle lost message before EOB
From: Marc Kleine-Budde @ 2013-10-31 19:24 UTC (permalink / raw)
  To: netdev
  Cc: davem, linux-can, kernel, Markus Pargmann, linux-stable,
	Marc Kleine-Budde
In-Reply-To: <1383247490-26984-1-git-send-email-mkl@pengutronix.de>

From: Markus Pargmann <mpa@pengutronix.de>

If we handle end of block messages with higher priority than a lost message,
we can run into an endless interrupt loop.

This is reproducable with a am335x processor and "cansequence -r" at 1Mbit.
As soon as we loose a packet we can't escape from an interrupt loop.

This patch fixes the problem by handling lost packets before EOB packets.

Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/c_can/c_can.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
index a668cd4..e3fc07c 100644
--- a/drivers/net/can/c_can/c_can.c
+++ b/drivers/net/can/c_can/c_can.c
@@ -814,9 +814,6 @@ static int c_can_do_rx_poll(struct net_device *dev, int quota)
 			msg_ctrl_save = priv->read_reg(priv,
 					C_CAN_IFACE(MSGCTRL_REG, 0));
 
-			if (msg_ctrl_save & IF_MCONT_EOB)
-				return num_rx_pkts;
-
 			if (msg_ctrl_save & IF_MCONT_MSGLST) {
 				c_can_handle_lost_msg_obj(dev, 0, msg_obj);
 				num_rx_pkts++;
@@ -824,6 +821,9 @@ static int c_can_do_rx_poll(struct net_device *dev, int quota)
 				continue;
 			}
 
+			if (msg_ctrl_save & IF_MCONT_EOB)
+				return num_rx_pkts;
+
 			if (!(msg_ctrl_save & IF_MCONT_NEWDAT))
 				continue;
 
-- 
1.8.4.rc3

^ permalink raw reply related

* [PATCH 2/2] can: kvaser_usb: fix usb endpoints detection
From: Marc Kleine-Budde @ 2013-10-31 19:24 UTC (permalink / raw)
  To: netdev
  Cc: davem, linux-can, kernel, Olivier Sobrie, linux-stable,
	Marc Kleine-Budde
In-Reply-To: <1383247490-26984-1-git-send-email-mkl@pengutronix.de>

From: Olivier Sobrie <olivier@sobrie.be>

Some devices, like the Kvaser Memorator Professional, have several bulk in
endpoints. Only the first one found must be used by the driver. The same holds
for the bulk out endpoint. The official Kvaser driver (leaf) was used as
reference for this patch.

Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: Olivier Sobrie <olivier@sobrie.be>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/kvaser_usb.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
index 3b95465..4b2d5ed 100644
--- a/drivers/net/can/usb/kvaser_usb.c
+++ b/drivers/net/can/usb/kvaser_usb.c
@@ -1544,9 +1544,9 @@ static int kvaser_usb_init_one(struct usb_interface *intf,
 	return 0;
 }
 
-static void kvaser_usb_get_endpoints(const struct usb_interface *intf,
-				     struct usb_endpoint_descriptor **in,
-				     struct usb_endpoint_descriptor **out)
+static int kvaser_usb_get_endpoints(const struct usb_interface *intf,
+				    struct usb_endpoint_descriptor **in,
+				    struct usb_endpoint_descriptor **out)
 {
 	const struct usb_host_interface *iface_desc;
 	struct usb_endpoint_descriptor *endpoint;
@@ -1557,12 +1557,18 @@ static void kvaser_usb_get_endpoints(const struct usb_interface *intf,
 	for (i = 0; i < iface_desc->desc.bNumEndpoints; ++i) {
 		endpoint = &iface_desc->endpoint[i].desc;
 
-		if (usb_endpoint_is_bulk_in(endpoint))
+		if (!*in && usb_endpoint_is_bulk_in(endpoint))
 			*in = endpoint;
 
-		if (usb_endpoint_is_bulk_out(endpoint))
+		if (!*out && usb_endpoint_is_bulk_out(endpoint))
 			*out = endpoint;
+
+		/* use first bulk endpoint for in and out */
+		if (*in && *out)
+			return 0;
 	}
+
+	return -ENODEV;
 }
 
 static int kvaser_usb_probe(struct usb_interface *intf,
@@ -1576,8 +1582,8 @@ static int kvaser_usb_probe(struct usb_interface *intf,
 	if (!dev)
 		return -ENOMEM;
 
-	kvaser_usb_get_endpoints(intf, &dev->bulk_in, &dev->bulk_out);
-	if (!dev->bulk_in || !dev->bulk_out) {
+	err = kvaser_usb_get_endpoints(intf, &dev->bulk_in, &dev->bulk_out);
+	if (err) {
 		dev_err(&intf->dev, "Cannot get usb endpoint(s)");
 		return err;
 	}
-- 
1.8.4.rc3

^ permalink raw reply related

* pull-request: can 2013-10-31
From: Marc Kleine-Budde @ 2013-10-31 19:24 UTC (permalink / raw)
  To: netdev; +Cc: davem, linux-can, kernel

Hello David,

I have two late fixes for the v3.12 release:

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.

regards,
Marc

---


The following changes since commit c17cb8b55b104c549aa20a72fa44141ad2c65ec2:

  doc:net: Fix typo in Documentation/networking (2013-10-30 17:10:20 -0400)

are available in the git repository at:

  git://gitorious.org/linux-can/linux-can.git fixes-for-3.12

for you to fetch changes up to 896e23bd04ea50a146dffd342e2f96180f0812a5:

  can: kvaser_usb: fix usb endpoints detection (2013-10-31 20:20:23 +0100)

----------------------------------------------------------------
Markus Pargmann (1):
      can: c_can: Fix RX message handling, handle lost message before EOB

Olivier Sobrie (1):
      can: kvaser_usb: fix usb endpoints detection

 drivers/net/can/c_can/c_can.c    |  6 +++---
 drivers/net/can/usb/kvaser_usb.c | 20 +++++++++++++-------
 2 files changed, 16 insertions(+), 10 deletions(-)

^ permalink raw reply

* Re: [PATCH net-next RFC 1/5] xen-netback: Introduce TX grant map definitions
From: Zoltan Kiss @ 2013-10-31 19:33 UTC (permalink / raw)
  To: Zoltan Kiss, ian.campbell, wei.liu2, xen-devel, netdev,
	linux-kernel, jonathan.davies
In-Reply-To: <1383094220-14775-2-git-send-email-zoltan.kiss@citrix.com>

On 30/10/13 00:50, Zoltan Kiss wrote:
> +void xenvif_zerocopy_callback(struct ubuf_info *ubuf, bool zerocopy_success)
> +{
> +	unsigned long flags;
> +	pending_ring_idx_t index;
> +	u16 pending_idx = ubuf->desc;
> +	struct pending_tx_info *temp =
> +		container_of(ubuf, struct pending_tx_info, callback_struct);
> +	struct xenvif *vif =
> +		container_of(temp - pending_idx, struct xenvif,
> +			pending_tx_info[0]);
> +
> +	spin_lock_irqsave(&vif->dealloc_lock, flags);
> +	do {
> +		pending_idx = ubuf->desc;
> +		ubuf = (struct ubuf_info *) ubuf->ctx;
> +		index = pending_index(vif->dealloc_prod);
> +		vif->dealloc_ring[index] = pending_idx;
> +		/* Sync with xenvif_tx_action_dealloc:
> +		 * insert idx then incr producer.
> +		 */
> +		smp_wmb();
> +		vif->dealloc_prod++;
> +		napi_schedule(&vif->napi);
> +	} while (ubuf);
> +	spin_unlock_irqrestore(&vif->dealloc_lock, flags);
> +}

Another possible place for improvement is the placing of napi_schedule. 
Now it get called after every fragment, which is probably suboptimal. I 
think it's likely that the vif thread can't finish one dealloc faster 
than one iteration of this while loop.
Another idea is to place it after the while, so it get called only once, 
but in the meantime the thread doesn't have chance to start working on 
the deallocs.
A compromise might be to do it once in the first iteration of the loop, 
and then once after the loop to make sure the thread knows about the 
dealloc.
Thoughts?

Zoli

^ permalink raw reply

* Re: [Xen-devel] [PATCH net-next RFC 3/5] xen-netback: Remove old TX grant copy definitons
From: Zoltan Kiss @ 2013-10-31 19:46 UTC (permalink / raw)
  To: Jan Beulich
  Cc: ian.campbell, jonathan.davies, wei.liu2, xen-devel, linux-kernel,
	netdev
In-Reply-To: <5270E1F902000078000FDFCA@nat28.tlf.novell.com>

On 30/10/13 09:39, Jan Beulich wrote:
>>>> On 30.10.13 at 01:50, Zoltan Kiss <zoltan.kiss@citrix.com> wrote:
>> These became obsolate with grant mapping.
>
> I didn't look at this in detail, but I'm surprised you can get away
> without any copying: For one, the header part needs copying
> anyway, so you'd pointlessly map and then copy it if it's small
> enough.
Yep, that's a further plan for optimization. I think I will add that as 
a separate patch to the series later. But that doesn't necessarily needs 
these definitions, let's see that later.

> And second you need to be prepared for the frontend
> to hand you more slots than you can fit in MAX_SKB_FRAGS
> (namely when MAX_SKB_FRAGS < XEN_NETIF_NR_SLOTS_MIN),
> which you can't handle with mapping alone afaict.
Oh, I was not aware of this problem. And indeed, the trivial solution is 
to keep the grant copy methods for such kind of packets, however that 
sounds quite nasty.
My another idea is to use skb_shinfo(skb)->frag_list for such packets, 
so the stack will see it as a fragmented IP packet. It might be less 
efficient than coalescing them into one skb during grant copy at first 
place, but probably a cleaner solution. If we don't care that much about 
the performance of such guests, it might be a better solution.
But I don't know that closely the IP fragmentation ideas, so it might be 
a bad idea. I'm happy to hear comments from people who have more 
experience with that.

Zoli

^ permalink raw reply

* Re: [PATCH net-next RFC 4/5] xen-netback: Fix indentations
From: Zoltan Kiss @ 2013-10-31 19:48 UTC (permalink / raw)
  To: Wei Liu; +Cc: ian.campbell, xen-devel, netdev, linux-kernel, jonathan.davies
In-Reply-To: <20131030111312.GN5221@zion.uk.xensource.com>

On 30/10/13 11:13, Wei Liu wrote:
> On Wed, Oct 30, 2013 at 12:50:19AM +0000, Zoltan Kiss wrote:
>> I've left intentionally these indentations in this way, to improve readability of previous patches.
>>
>
> Apparently this doesn't deserve a single patch -- please squash it when
> you post non-RFC series.
>
> Wei.
>
Yep, I just want to keep them separately until the series get it's final 
form.

Zoli

^ permalink raw reply

* pull-request: can-next 2013-10-31
From: Marc Kleine-Budde @ 2013-10-31 20:06 UTC (permalink / raw)
  To: netdev; +Cc: David Miller, linux-can@vger.kernel.org, kernel@pengutronix.de

[-- Attachment #1: Type: text/plain, Size: 1706 bytes --]

Hello David,

here's a pull request for net-next.

It includes a patch by Oliver Hartkopp et al. that adds documentation
for the broadcast manager to Documentation/networking/can.txt. Three
patches by me that clean up the netlink handling code in the CAN core.
And another patch that removes a not needed function from the ti_hecc
driver.

regards,
Marc

---

The following changes since commit ba4865027c11d7ac8e5a33e0624dd415caab2027:

  ipv6: remove the unnecessary statement in find_match() (2013-10-30 17:06:51 -0400)

are available in the git repository at:

  git://gitorious.org/linux-can/linux-can-next.git for-davem

for you to fetch changes up to 26896fd98130cdda73b19c75849a0d25cb850cb8:

  can: ti_hecc: remove priv->can.do_get_state() callback (2013-10-31 20:55:16 +0100)

----------------------------------------------------------------
Marc Kleine-Budde (4):
      can: dev: sort can_get_size() by IFLA_CAN_*
      can: dev: sort can_fill_info() by IFLA_CAN_*
      can: dev: sort can_changelink() by IFLA_CAN_*
      can: ti_hecc: remove priv->can.do_get_state() callback

Oliver Hartkopp (1):
      can: add broadcast manager documentation

 Documentation/networking/can.txt | 217 +++++++++++++++++++++++++++++++++++++++
 drivers/net/can/dev.c            |  67 ++++++------
 drivers/net/can/ti_hecc.c        |  10 --
 3 files changed, 249 insertions(+), 45 deletions(-)

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

^ permalink raw reply

* Re: [net-next PATCH] net: codel: Avoid undefined behavior from signed overflow
From: Jesper Dangaard Brouer @ 2013-10-31 20:40 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Jesper Dangaard Brouer, netdev, Paul E. McKenney, Dave Taht
In-Reply-To: <1383232241.4857.73.camel@edumazet-glaptop.roam.corp.google.com>

On Thu, 31 Oct 2013 08:10:41 -0700
Eric Dumazet <eric.dumazet@gmail.com> wrote:

> On Thu, 2013-10-31 at 15:15 +0100, Jesper Dangaard Brouer wrote:
> 
> > Okay, I'll cook up another patch, after work.
> > 
> > Adding all the typecheck() stuff, just bloats the code.
> > 
> > Would it be better/okay just to do?:
> >  (s32)((u32)(a) - (u32)(b)) > 0)
> > 
> > 
> 
> What about using the existing codel types ?

Hmm, I would be okay to use codel types for typecheck(), but I don't
like the approach below, because we are hiding a typecast.  This just
makes the code harder to read/understand. An explicit cast shows that
we are doing something nasty, on purpose here.

I would rather keep as close as possible to include/linux/jiffies.h,
because I want readers to be-able to spot this pattern.


> diff --git a/include/net/codel.h b/include/net/codel.h
> index 389cf62..89a7781 100644
> --- a/include/net/codel.h
> +++ b/include/net/codel.h
> @@ -72,7 +72,12 @@ static inline codel_time_t codel_get_time(void)
>  	return ns >> CODEL_SHIFT;
>  }
>  
> -#define codel_time_after(a, b)		((s32)(a) - (s32)(b) > 0)
> +static inline bool codel_time_after(codel_time_t a, codel_time_t b)
> +{
> +	codel_tdiff_t delta = a - b;
> +
> +	return delta >= 0;
> +}
>  #define codel_time_after_eq(a, b)	((s32)(a) - (s32)(b) >= 0)
>  #define codel_time_before(a, b)		((s32)(a) - (s32)(b) < 0)
>  #define codel_time_before_eq(a, b)	((s32)(a) - (s32)(b) <= 0)
> 
> 
> You need of course something similar for all variants.
> 
> 



-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Sr. Network Kernel Developer at Red Hat
  Author of http://www.iptv-analyzer.org
  LinkedIn: http://www.linkedin.com/in/brouer

^ permalink raw reply

* [net-next PATCH V2] net: codel: Avoid undefined behavior from signed overflow
From: Jesper Dangaard Brouer @ 2013-10-31 21:10 UTC (permalink / raw)
  To: netdev; +Cc: Eric Dumazet, Paul E. McKenney, Dave Taht, Jesper Dangaard Brouer

From: Jesper Dangaard Brouer <netoptimizer@brouer.com>

As described in commit 5a581b367 (jiffies: Avoid undefined
behavior from signed overflow), according to the C standard
3.4.3p3, overflow of a signed integer results in undefined
behavior.

To fix this, do as the above commit, and do an unsigned
subtraction, and interpreting the result as a signed
two's-complement number.  This is based on the theory from
RFC 1982 and is nicely described in wikipedia here:
 https://en.wikipedia.org/wiki/Serial_number_arithmetic#General_Solution

A side-note, I have seen practical issues with the previous logic
when dealing with 16-bit, on a 64-bit machine (gcc version
4.4.5). This were 32-bit, which I have not observed issues with.

Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Signed-off-by: Jesper Dangaard Brouer <netoptimizer@brouer.com>
---

 include/net/codel.h |   19 +++++++++++++++----
 1 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/include/net/codel.h b/include/net/codel.h
index 389cf62..3b04ff5 100644
--- a/include/net/codel.h
+++ b/include/net/codel.h
@@ -72,10 +72,21 @@ static inline codel_time_t codel_get_time(void)
 	return ns >> CODEL_SHIFT;
 }
 
-#define codel_time_after(a, b)		((s32)(a) - (s32)(b) > 0)
-#define codel_time_after_eq(a, b)	((s32)(a) - (s32)(b) >= 0)
-#define codel_time_before(a, b)		((s32)(a) - (s32)(b) < 0)
-#define codel_time_before_eq(a, b)	((s32)(a) - (s32)(b) <= 0)
+/* Dealing with timer wrapping, according to RFC 1982, as desc in wikipedia:
+ *  https://en.wikipedia.org/wiki/Serial_number_arithmetic#General_Solution
+ * codel_time_after(a,b) returns true if the time a is after time b.
+ */
+#define codel_time_after(a, b)						\
+	(typecheck(codel_time_t, a) &&					\
+	 typecheck(codel_time_t, b) &&					\
+	 ((s32)((a) - (b)) > 0))
+#define codel_time_before(a, b) 	codel_time_after(b, a)
+
+#define codel_time_after_eq(a, b)					\
+	(typecheck(codel_time_t, a) &&					\
+	 typecheck(codel_time_t, b) &&					\
+	 ((s32)((a) - (b)) >= 0))
+#define codel_time_before_eq(a, b)	codel_time_after_eq(b, a)
 
 /* Qdiscs using codel plugin must use codel_skb_cb in their own cb[] */
 struct codel_skb_cb {

^ permalink raw reply related

* Re: [net-next PATCH] net: codel: Avoid undefined behavior from signed overflow
From: Jesper Dangaard Brouer @ 2013-10-31 21:53 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: Jesper Dangaard Brouer, netdev, Eric Dumazet, Paul E. McKenney,
	Dave Taht, Eilon Greenstein
In-Reply-To: <1383161748.1601.24.camel@bwh-desktop.uk.level5networks.com>

On Wed, 30 Oct 2013 19:35:48 +0000
Ben Hutchings <bhutchings@solarflare.com> wrote:

> On Wed, 2013-10-30 at 18:23 +0100, Jesper Dangaard Brouer wrote:
> > From: Jesper Dangaard Brouer <netoptimizer@brouer.com>
> > 
> > As described in commit 5a581b367 (jiffies: Avoid undefined
> > behavior from signed overflow), according to the C standard
> > 3.4.3p3, overflow of a signed integer results in undefined
> > behavior.
> [...]
> 
> According to the real processors that Linux runs on, signed arithmetic
> uses 2's complement representation and overflow wraps accordingly.  And
> we rely on that behaviour in many places, so we use
> '-fno-strict-overflow' to tell gcc not to assume we avoid signed
> overflow.  (There is also '-fwrapv' which tells gcc to assume the
> processor behaves this way, but shouldn't it already know how the target
> machine works?)

For 16-bit I have tested that is fails, and that it does not help to
use the compiler flag: '-fno-strict-overflow' or '-fwrapv'. (this was
userspace test code, so I might be missing some kernel compiler options
that would make this work for 16-bit, but I doubt it)

#define works_u16_time_after(a,b)			\
	(typecheck(u_int16_t, a) &&		\
	 typecheck(u_int16_t, b) &&		\
	 ((int16_t)((b) - (a)) < 0))

#define bad_u16_time_after(a,b)			\
	(typecheck(u_int16_t, a) &&		\
	 typecheck(u_int16_t, b) &&		\
	 (((int16_t)(b) - (int16_t)(a)) < 0))


The bnx2x have a wrong/dangerup construct:

 File: drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
 #define SUB_S16(a, b)		(s16)((s16)(a) - (s16)(b))
 #define SUB_S32(a, b)		(s32)((s32)(a) - (s32)(b))

I have tested this case, and it surprisingly works, due to the outer
(s16) cast I believe.

I think this should/could be fixed like:

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h b/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
index 4e01c57..8969733 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
@@ -838,8 +838,8 @@ static inline bool bnx2x_fp_ll_polling(struct bnx2x_fastpath *fp)
 #define RCQ_TH_HI(bp)          (RCQ_TH_LO(bp) + DROPLESS_FC_HEADROOM)
 
 /* This is needed for determining of last_max */
-#define SUB_S16(a, b)          (s16)((s16)(a) - (s16)(b))
-#define SUB_S32(a, b)          (s32)((s32)(a) - (s32)(b))
+#define SUB_S16(a, b)          (s16)((u16)(a) - (u16)(b))
+#define SUB_S32(a, b)          (s32)((u32)(a) - (u32)(b))
 
 #define BNX2X_SWCID_SHIFT      17
 #define BNX2X_SWCID_MASK       ((0x1 << BNX2X_SWCID_SHIFT) - 1)


-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Sr. Network Kernel Developer at Red Hat
  Author of http://www.iptv-analyzer.org
  LinkedIn: http://www.linkedin.com/in/brouer

^ permalink raw reply related

* Re: [PATCH net-next] tcp: enable sockets to use MSG_FASTOPEN by default
From: Eric Dumazet @ 2013-10-31 23:19 UTC (permalink / raw)
  To: Yuchung Cheng; +Cc: davem, edumazet, netdev, ncardwell, sivasankar
In-Reply-To: <1383236372-1317-1-git-send-email-ycheng@google.com>

On Thu, 2013-10-31 at 09:19 -0700, Yuchung Cheng wrote:
> Applications have started to use Fast Open (e.g., Chrome browser has
> such an optional flag) and the feature has gone through several
> generations of kernels since 3.7 with many real network tests. It's
> time to enable this flag by default for applications to test more
> conveniently and extensively.
> 
> Signed-off-by: Yuchung Cheng <ycheng@google.com>
> Signed-off-by: Neal Cardwell <ncardwell@google.com>
> ---

Acked-by: Eric Dumazet <edumazet@google.com>

^ permalink raw reply


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