Netdev List
 help / color / mirror / Atom feed
* [PATCH] cw1200: use msecs_to_jiffies for conversion
From: Nicholas Mc Guire @ 2015-02-04  7:39 UTC (permalink / raw)
  To: Solomon Peachy
  Cc: Kalle Valo, linux-wireless, netdev, linux-kernel,
	Nicholas Mc Guire

This is only an API consolidation to make things more readable.
Instances of  HZ / CONST  are replaced by appropriate msecs_to_jiffies().

Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
---

Converting milliseconds to jiffies by "val * HZ / 1000" or passing
HZ / 10 is technically OK but appropriate msecs_to_jiffies(val), respectively
msecs_to_jiffies(100) is the cleaner solution and handles all corner cases 
correctly. This is a minor API cleanup only.

Patch was only compile tested for x86_64_defconfig + CONFIG_CW1200=m

Patch is against 3.19.0-rc7 (localversion-next = -next-20150203)

 drivers/net/wireless/cw1200/scan.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/cw1200/scan.c b/drivers/net/wireless/cw1200/scan.c
index f2e276f..bff81b8 100644
--- a/drivers/net/wireless/cw1200/scan.c
+++ b/drivers/net/wireless/cw1200/scan.c
@@ -39,9 +39,9 @@ static int cw1200_scan_start(struct cw1200_common *priv, struct wsm_scan *scan)
 	cancel_delayed_work_sync(&priv->clear_recent_scan_work);
 	atomic_set(&priv->scan.in_progress, 1);
 	atomic_set(&priv->recent_scan, 1);
-	cw1200_pm_stay_awake(&priv->pm_state, tmo * HZ / 1000);
+	cw1200_pm_stay_awake(&priv->pm_state, msecs_to_jiffies(tmo));
 	queue_delayed_work(priv->workqueue, &priv->scan.timeout,
-			   tmo * HZ / 1000);
+			   msecs_to_jiffies(tmo));
 	ret = wsm_scan(priv, scan);
 	if (ret) {
 		atomic_set(&priv->scan.in_progress, 0);
@@ -386,8 +386,8 @@ void cw1200_probe_work(struct work_struct *work)
 	if (down_trylock(&priv->scan.lock)) {
 		/* Scan is already in progress. Requeue self. */
 		schedule();
-		queue_delayed_work(priv->workqueue,
-				   &priv->scan.probe_work, HZ / 10);
+		queue_delayed_work(priv->workqueue, &priv->scan.probe_work,
+				   msecs_to_jiffies(100));
 		mutex_unlock(&priv->conf_mutex);
 		return;
 	}
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH net-next] xps: fix xps for stacked devices
From: Eric Dumazet @ 2015-02-04  7:48 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Willem de Bruijn, Nandita Dukkipati, Yuchung Cheng

From: Eric Dumazet <edumazet@google.com>

A typical qdisc setup is the following :

bond0 : bonding device, using HTB hierarchy
eth1/eth2 : slaves, multiqueue NIC, using MQ + FQ qdisc

XPS allows to spread packets on specific tx queues, based on the cpu
doing the send.

Problem is that dequeues from bond0 qdisc can happen on random cpus,
due to the fact that qdisc_run() can dequeue a batch of packets.

CPUA -> queue packet P1 on bond0 qdisc, P1->ooo_okay=1
CPUA -> queue packet P2 on bond0 qdisc, P2->ooo_okay=0

CPUB -> dequeue packet P1 from bond0
        enqueue packet on eth1/eth2
CPUC -> dequeue packet P2 from bond0
        enqueue packet on eth1/eth2 using sk cache (ooo_okay is 0)

get_xps_queue() then might select wrong queue for P1, since current cpu
might be different than CPUA.

P2 might be sent on the old queue (stored in sk->sk_tx_queue_mapping),
if CPUC runs a bit faster (or CPUB spins a bit on qdisc lock)

Effect of this bug is TCP reorders, and more generally not optimal
TX queue placement. (A victim bulk flow can be migrated to the wrong TX
queue for a while)

To fix this, we have to record sender cpu number the first time
dev_queue_xmit() is called for one tx skb.

We can union napi_id (used on receive path) and sender_cpu,
granted we clear sender_cpu in skb_scrub_packet() (credit to Willem for
this union idea)

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Willem de Bruijn <willemb@google.com>
Cc: Nandita Dukkipati <nanditad@google.com>
Cc: Yuchung Cheng <ycheng@google.com>
---
 include/linux/skbuff.h    |    7 +++++--
 net/core/flow_dissector.c |    7 ++++++-
 net/core/skbuff.c         |    4 ++++
 3 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 85ab7d72b54c2f269812015b19544674bc6dcd72..2748ff63914438268458246adb165e61ed892656 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -626,8 +626,11 @@ struct sk_buff {
 	__u32			hash;
 	__be16			vlan_proto;
 	__u16			vlan_tci;
-#ifdef CONFIG_NET_RX_BUSY_POLL
-	unsigned int	napi_id;
+#if defined(CONFIG_NET_RX_BUSY_POLL) || defined(CONFIG_XPS)
+	union {
+		unsigned int	napi_id;
+		unsigned int	sender_cpu;
+	};
 #endif
 #ifdef CONFIG_NETWORK_SECMARK
 	__u32			secmark;
diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index beb83d1ac1c688d7b593a5cad85236b6d94c3106..2c35c02a931e227fa368cd346873596d4b037a3d 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -422,7 +422,7 @@ static inline int get_xps_queue(struct net_device *dev, struct sk_buff *skb)
 	dev_maps = rcu_dereference(dev->xps_maps);
 	if (dev_maps) {
 		map = rcu_dereference(
-		    dev_maps->cpu_map[raw_smp_processor_id()]);
+		    dev_maps->cpu_map[skb->sender_cpu - 1]);
 		if (map) {
 			if (map->len == 1)
 				queue_index = map->queues[0];
@@ -468,6 +468,11 @@ struct netdev_queue *netdev_pick_tx(struct net_device *dev,
 {
 	int queue_index = 0;
 
+#ifdef CONFIG_XPS
+	if (skb->sender_cpu == 0)
+		skb->sender_cpu = raw_smp_processor_id() + 1;
+#endif
+
 	if (dev->real_num_tx_queues != 1) {
 		const struct net_device_ops *ops = dev->netdev_ops;
 		if (ops->ndo_select_queue)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index a5bff2767f15abe09b5f0d0a3bfecfb5775a4e64..88c613eab142962dc44f2075378fce0b94349e8e 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -825,6 +825,9 @@ static void __copy_skb_header(struct sk_buff *new, const struct sk_buff *old)
 #ifdef CONFIG_NET_RX_BUSY_POLL
 	CHECK_SKB_FIELD(napi_id);
 #endif
+#ifdef CONFIG_XPS
+	CHECK_SKB_FIELD(sender_cpu);
+#endif
 #ifdef CONFIG_NET_SCHED
 	CHECK_SKB_FIELD(tc_index);
 #ifdef CONFIG_NET_CLS_ACT
@@ -4169,6 +4172,7 @@ void skb_scrub_packet(struct sk_buff *skb, bool xnet)
 	skb->ignore_df = 0;
 	skb_dst_drop(skb);
 	skb->mark = 0;
+	skb->sender_cpu = 0;
 	skb_init_secmark(skb);
 	secpath_reset(skb);
 	nf_reset(skb);

^ permalink raw reply related

* Re: [PATCH net-next] cxgb4: Add low latency socket busy_poll support
From: Eric Dumazet @ 2015-02-04  7:50 UTC (permalink / raw)
  To: Kumar Sanghvi
  Cc: Hariprasad Shenai, netdev, davem, leedom, anish, nirranjan,
	praveenm
In-Reply-To: <20150204062137.GD30631@kumar-pc.asicdesigners.com>

On Wed, 2015-02-04 at 12:21 +0600, Kumar Sanghvi wrote:
> On Tuesday, February 02/03/15, 2015 at 21:58:29 -0800, Eric Dumazet wrote:
> > On Tue, 2015-02-03 at 10:50 +0530, Hariprasad Shenai wrote:
> > 
> > > @@ -1978,9 +2016,13 @@ static int napi_rx_handler(struct napi_struct *napi, int budget)
> > >  {
> > >  	unsigned int params;
> > >  	struct sge_rspq *q = container_of(napi, struct sge_rspq, napi);
> > > -	int work_done = process_responses(q, budget);
> > > +	int work_done = 0;
> > >  	u32 val;
> > >  
> > > +	if (!cxgb_poll_lock_napi(q))
> > > +		return work_done;
> > > +
> > 
> > 
> > This is very suspicious. Please take a look at commits
> > 
> > 24e579c8898aa641 ("bnx2x: fix napi poll return value for repoll") 
> > f104fedc0da126ab ("enic: fix rx napi poll return value")
> > 
> > for context.
> 
> Thanks Eric for pointing on this.
> So, this probably needs change after commit d75b1ade567ffab ("net: less interrupt masking in NAPI"), or
> is there something else that is missed ?

Right, you probably need to return budget here, not 0 (work_done == 0 at
this stage)

^ permalink raw reply

* Re: [PATCH net-next] bridge: Let bridge not age 'externally' learnt FDB entries, they are removed when 'external' entity notifies the aging
From: Siva Mannem @ 2015-02-04  8:02 UTC (permalink / raw)
  To: roopa; +Cc: Netdev, Scott Feldman, Jiri Pirko
In-Reply-To: <54D0E521.6070100@cumulusnetworks.com>

On Tue, Feb 3, 2015 at 8:41 PM, roopa <roopa@cumulusnetworks.com> wrote:
> On 2/2/15, 9:21 AM, Siva Mannem wrote:
>>
>>   When 'learned_sync' flag is turned on, the offloaded switch
>>   port syncs learned MAC addresses to bridge's FDB via switchdev notifier
>>   (NETDEV_SWITCH_FDB_ADD). Currently, FDB entries learnt via this
>> mechanism are
>>   wrongly being deleted by bridge aging logic. This patch ensures that FDB
>>   entries synced from offloaded switch ports are not deleted by bridging
>> logic.
>>   Such entries can only be deleted via switchdev notifier
>>   (NETDEV_SWITCH_FDB_DEL).
>
>
> Your patch seems right and maintains symmetry for fdb add/del of externally
> learnt entries.
> However, this could be made configurable. I think some drivers may rely on
> bridge driver aging these entries (The default setting needs more thought).
> I am not sure what rocker does (CC'ed rocker maintainers). But, our driver
> does rely on the bridge driver aging these entries by default.

added_by_external_learn flag is only set for entries learned via
switchdev notifier
(NETDEV_SWITCH_FDB_ADD) and rocker is the only driver using these notifiers.
 I see that rocker is deleting the entries via switchdev notifier
(NETDEV_SWITCH_FDB_DEL).
This mechanism is only used by drivers when learned_sync is turned on by user.

$ sudo bridge link set dev swp1 learning_sync on self

Am I missing something here?

>
>>
>> Signed-off-by: Siva Mannem <siva.mannem.lnx@gmail.com>
>> ---
>>   net/bridge/br_fdb.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
>> index 08bf04b..6eb94b5 100644
>> --- a/net/bridge/br_fdb.c
>> +++ b/net/bridge/br_fdb.c
>> @@ -280,7 +280,7 @@ void br_fdb_cleanup(unsigned long _data)
>>                 hlist_for_each_entry_safe(f, n, &br->hash[i], hlist) {
>>                         unsigned long this_timer;
>> -                       if (f->is_static)
>> +                       if (f->is_static || f->added_by_external_learn)
>>                                 continue;
>>                         this_timer = f->updated + delay;
>>                         if (time_before_eq(this_timer, jiffies))
>
>



-- 
Regards,
Siva Mannem.

^ permalink raw reply

* Re: Invalid timestamp? causing tight ack loop (hundreds of thousands of packets / sec)
From: Eric Dumazet @ 2015-02-04  8:03 UTC (permalink / raw)
  To: Avery Fay; +Cc: netdev, Neal Cardwell
In-Reply-To: <CAO-X30uMA7=DTm5KqqvYC5RVTM0bg9pKBO1nX1+6x_2pF_fWfA@mail.gmail.com>

On Tue, 2015-02-03 at 22:50 -0800, Avery Fay wrote:
> Hello,
> 
> Let me say first: if there's a better place to ask this, please point
> me in that direction.
> 
> We've been having huge packets / sec spikes in the past few days.
> After some investigation, it looks like single connections are getting
> stuck in a loop (see tcpdump below). Each "stuck" connection will
> generate about 200kpps. It looks like our side is rejecting packets
> with "packets rejects in established connections because of timestamp"
> from netstat -s (internally PAWSEstab counter) and then generating an
> additional packet that we send out. All of these connections originate
> from georgia tech, but so far (not completely verified) it doesn't
> seem like there's any pattern to the client/os other than the fact
> that they're trying to make an https request to us.
> 
> As a temporary countermeasure, we've disabled net.ipv4.tcp_timestamps,
> which solves the immediate problem.
> 
> Our server is 174.36.240.86 running Ubuntu 12.04 with kernel 3.13.0-35-generic
> 
> The client is 128.61.57.205 and in this case almost certainly has user
> agent (we found successful requests 10 seconds before the tcpdump with
> same ip): Dalvik/2.1.0 (Linux; U; Android 5.0; XT1095
> Build/LXE22.46-11)
> 
> Beginning of tcpdump:

...

> 
> At this point, it just repeats until some timeout is hit. I haven't
> timed it, but probably one or two minutes.
> 
> I guess I have a few questions:
> 
> 1.) What's going on here? It looks like maybe there's some packet loss
> and then connection termination gets stuck in a loop because the
> client timestamp went down?
> 2.) Is there a better way to mitigate this other than disabling
> tcp_timestamps or blocking gatech ips?
> 3.) Is this our problem (ok, obviously our problem since we're
> affected but...), a kernel problem, or a gatech problem?
> 
> I'd really appreciate any help on this,

Would you have a pcap file instead ?

It looks a middlebox is broken, I dont think Android could possibly send
a frame with no payload, but with Push flag.

Neal has some patches that add a rate limiting on DACKS, that we might
upstream. (per socket rate limiting of 2 DACK per second)

Thanks

^ permalink raw reply

* [PATCH 1/3] wireless: orinoco: orinoco_plx use msecs_to_jiffies for conversion
From: Nicholas Mc Guire @ 2015-02-04  8:07 UTC (permalink / raw)
  To: Kalle Valo
  Cc: Benoit Taine, Bjorn Helgaas,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Nicholas Mc Guire

This is only an API consolidation and should make things more readable
it replaces var * HZ / 1000 by msecs_to_jiffies(var).

Signed-off-by: Nicholas Mc Guire <hofrat-Q945KHDl0DbYtjvyW6yDsg@public.gmane.org>
---

Converting milliseconds to jiffies by "val * HZ / 1000" is technically
ok but msecs_to_jiffies(val) is the cleaner solution and handles all
corner cases correctly. This is a minor API cleanup only.

Patch was compile-tested only for x86_64_defconfig + CONFIG_HERMES=m
CONFIG_PLX_HERMES=m

Patch is against 3.19.0-rc7 (localversion-next is -next-20150203)

 drivers/net/wireless/orinoco/orinoco_plx.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/orinoco/orinoco_plx.c b/drivers/net/wireless/orinoco/orinoco_plx.c
index b8f6e5c..8b04523 100644
--- a/drivers/net/wireless/orinoco/orinoco_plx.c
+++ b/drivers/net/wireless/orinoco/orinoco_plx.c
@@ -121,7 +121,7 @@ static int orinoco_plx_cor_reset(struct orinoco_private *priv)
 	mdelay(1);
 
 	/* Just in case, wait more until the card is no longer busy */
-	timeout = jiffies + (PLX_RESET_TIME * HZ / 1000);
+	timeout = jiffies + msecs_to_jiffies(PLX_RESET_TIME);
 	reg = hermes_read_regn(hw, CMD);
 	while (time_before(jiffies, timeout) && (reg & HERMES_CMD_BUSY)) {
 		mdelay(1);
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH 2/3] wireless: orinoco: orinoco_pci use msecs_to_jiffies for conversion
From: Nicholas Mc Guire @ 2015-02-04  8:07 UTC (permalink / raw)
  To: Kalle Valo
  Cc: Benoit Taine, Bjorn Helgaas, linux-wireless, netdev, linux-kernel,
	Nicholas Mc Guire
In-Reply-To: <1423037261-20193-1-git-send-email-hofrat@osadl.org>

This is only an API consolidation and should make things more readable
it replaces var * HZ / 1000 by msecs_to_jiffies(var).

Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
---

Converting milliseconds to jiffies by "val * HZ / 1000" is technically
ok but msecs_to_jiffies(val) is the cleaner solution and handles all
corner cases correctly. This is a minor API cleanup only.

Patch was compile-tested only for x86_64_defconfig + CONFIG_HERMES=m
CONFIG_HERMES_PRISM=y, CONFIG_PCI_HERMES=m

Patch is against 3.19.0-rc7 (localversion-next is -next-20150203)

 drivers/net/wireless/orinoco/orinoco_pci.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/orinoco/orinoco_pci.c b/drivers/net/wireless/orinoco/orinoco_pci.c
index b6bdad6..74219d5 100644
--- a/drivers/net/wireless/orinoco/orinoco_pci.c
+++ b/drivers/net/wireless/orinoco/orinoco_pci.c
@@ -94,7 +94,7 @@ static int orinoco_pci_cor_reset(struct orinoco_private *priv)
 	mdelay(HERMES_PCI_COR_OFFT);
 
 	/* The card is ready when it's no longer busy */
-	timeout = jiffies + (HERMES_PCI_COR_BUSYT * HZ / 1000);
+	timeout = jiffies + msecs_to_jiffies(HERMES_PCI_COR_BUSYT);
 	reg = hermes_read_regn(hw, CMD);
 	while (time_before(jiffies, timeout) && (reg & HERMES_CMD_BUSY)) {
 		mdelay(1);
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH 3/3] wireless: orinoco: orinoco_tmd use msecs_to_jiffies for conversion
From: Nicholas Mc Guire @ 2015-02-04  8:07 UTC (permalink / raw)
  To: Kalle Valo
  Cc: Benoit Taine, Bjorn Helgaas, linux-wireless, netdev, linux-kernel,
	Nicholas Mc Guire
In-Reply-To: <1423037261-20193-1-git-send-email-hofrat@osadl.org>

This is only an API consolidation and should make things more readable
it replaces var * HZ / 1000 by msecs_to_jiffies(var).

Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
---

Converting milliseconds to jiffies by "val * HZ / 1000" is technically
ok but msecs_to_jiffies(val) is the cleaner solution and handles all
corner cases correctly. This is a minor API cleanup only.

Patch was compile-tested only for x86_64_defconfig + CONFIG_HERMES=m
CONFIG_TMD_HERMES=m

Patch is against 3.19.0-rc7 (localversion-next is -next-20150203)

 drivers/net/wireless/orinoco/orinoco_tmd.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/orinoco/orinoco_tmd.c b/drivers/net/wireless/orinoco/orinoco_tmd.c
index 79d0e33..20ce569 100644
--- a/drivers/net/wireless/orinoco/orinoco_tmd.c
+++ b/drivers/net/wireless/orinoco/orinoco_tmd.c
@@ -71,7 +71,7 @@ static int orinoco_tmd_cor_reset(struct orinoco_private *priv)
 	mdelay(1);
 
 	/* Just in case, wait more until the card is no longer busy */
-	timeout = jiffies + (TMD_RESET_TIME * HZ / 1000);
+	timeout = jiffies + msecs_to_jiffies(TMD_RESET_TIME);
 	reg = hermes_read_regn(hw, CMD);
 	while (time_before(jiffies, timeout) && (reg & HERMES_CMD_BUSY)) {
 		mdelay(1);
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH RFC] ipv4 tcp: Use fine granularity to increase probe_size for tcp pmtu
From: Fan Du @ 2015-02-04  8:10 UTC (permalink / raw)
  To: netdev; +Cc: jesse, pshelar, dev, fengyuleidian0615

A couple of month ago, I proposed a fix for over-MTU-sized vxlan
packet loss at link[1], neither by fragmenting the tunnelled vxlan
packet, nor pushing back PMTU ICMP need fragmented message is 
accepted by community. The upstream workaround is by adjusting
guest mtu smaller or host mtu bigger, or by making virtio driver
auto-tuned guest mtu(no consensus by now). Note, gre tunnel also
suffer the over-MTU-sized packet loss.

While For TCPv4 case, this issue could be solved by using
Packetization Layer Path MTU Discovery which is defined as [3] 
from commit: 5d424d5a674f ("[TCP]: MTU probing").

echo 1 > /proc/sys/net/ipv4/tcp_mtu_probing

One drawback of tcp level mtu probing is:The original strategy is
double mss_cache for each probe, this is way too aggressive for 
over-MTU-sized vxlan packet loss issue from the performance result.
Also, the probing is characterized by tcp retransmission, which usual
taking 6 seconds from the first drop packet to normal connectivity
recovery.

By incrementing 25% of original mss_cache each time, performance
boost from ~1.3Gbits/s(mss_cache 1024Bytes) to ~1.55Gbits/s(
mss_cache 1250Bytes), more generic theme could be used there for
other tunnel technology.

No sure why tcp level mtu probing got disabled by default, any
historic known issues or pitfalls?

[1]: http://www.spinics.net/lists/netdev/msg306502.html
[2]: http://www.ietf.org/rfc/rfc4821.txt

Signed-off-by: Fan Du <fan.du@intel.com>
---
 net/ipv4/tcp_output.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 20ab06b..ab7e46b 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -1856,9 +1856,11 @@ static int tcp_mtu_probe(struct sock *sk)
 	    tp->rx_opt.num_sacks || tp->rx_opt.dsack)
 		return -1;
 
-	/* Very simple search strategy: just double the MSS. */
+	/* Very simple search strategy:
+	 * Increment 25% of orignal MSS forward
+	 */
 	mss_now = tcp_current_mss(sk);
-	probe_size = 2 * tp->mss_cache;
+	probe_size = (tp->mss_cache + (tp->mss_cache >> 2));
 	size_needed = probe_size + (tp->reordering + 1) * tp->mss_cache;
 	if (probe_size > tcp_mtu_to_mss(sk, icsk->icsk_mtup.search_high)) {
 		/* TODO: set timer for probe_converge_event */
-- 
1.7.1

^ permalink raw reply related

* Re: Invalid timestamp? causing tight ack loop (hundreds of thousands of packets / sec)
From: Avery Fay @ 2015-02-04  8:35 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev, Neal Cardwell
In-Reply-To: <1423036986.907.105.camel@edumazet-glaptop2.roam.corp.google.com>

Sure, https://dl.dropboxusercontent.com/u/9777748/loop.pcap.gz

Also, no idea if it helps, but here's the traceroute:

HOST: apibalancer-wdc-05                          Loss%   Snt   Last
Avg  Best  Wrst StDev
  1.|-- 184.173.130.1-static.reverse.softlayer.com   0.0%    10    0.2
  1.9   0.2   9.6   3.1
  2.|-- 208.43.118.164-static.reverse.softlayer.com  0.0%    10    0.2
  0.2   0.2   0.3   0.0
  3.|-- ae8.bbr02.eq01.wdc02.networklayer.com        0.0%    10    1.2
  1.3   1.1   2.6   0.5
  4.|-- ash-b1-link.telia.net                        0.0%    10    1.2
  4.3   1.1  12.7   5.0
  5.|-- ash-bb3-link.telia.net                       0.0%    10    1.2
  2.6   1.1  15.0   4.4
  6.|-- atl-bb1-link.telia.net                       0.0%    10   13.4
 13.3  13.3  13.4   0.0
  7.|-- 213.248.94.220                               0.0%    10   14.2
 14.2  14.2  14.4   0.1
  8.|-- 130.207.254.6                                0.0%    10   14.2
 14.2  14.1  14.3   0.1
    |  `|-- 130.207.254.185
  9.|-- gateway2-rtr.gatech.edu                      0.0%    10   14.2
 14.3  14.1  14.8   0.2
    |  `|-- 143.215.254.97
 10.|-- 143.215.254.97                               0.0%    10   14.6
 14.6  14.4  14.7   0.1
    |  `|-- 143.215.253.114
 11.|-- 143.215.253.114                              0.0%    10   15.1
 14.7  14.5  15.1   0.2
 12.|-- ???                                         100.0    10    0.0
  0.0   0.0   0.0   0.0

On Wed, Feb 4, 2015 at 12:03 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Tue, 2015-02-03 at 22:50 -0800, Avery Fay wrote:
>> Hello,
>>
>> Let me say first: if there's a better place to ask this, please point
>> me in that direction.
>>
>> We've been having huge packets / sec spikes in the past few days.
>> After some investigation, it looks like single connections are getting
>> stuck in a loop (see tcpdump below). Each "stuck" connection will
>> generate about 200kpps. It looks like our side is rejecting packets
>> with "packets rejects in established connections because of timestamp"
>> from netstat -s (internally PAWSEstab counter) and then generating an
>> additional packet that we send out. All of these connections originate
>> from georgia tech, but so far (not completely verified) it doesn't
>> seem like there's any pattern to the client/os other than the fact
>> that they're trying to make an https request to us.
>>
>> As a temporary countermeasure, we've disabled net.ipv4.tcp_timestamps,
>> which solves the immediate problem.
>>
>> Our server is 174.36.240.86 running Ubuntu 12.04 with kernel 3.13.0-35-generic
>>
>> The client is 128.61.57.205 and in this case almost certainly has user
>> agent (we found successful requests 10 seconds before the tcpdump with
>> same ip): Dalvik/2.1.0 (Linux; U; Android 5.0; XT1095
>> Build/LXE22.46-11)
>>
>> Beginning of tcpdump:
>
> ...
>
>>
>> At this point, it just repeats until some timeout is hit. I haven't
>> timed it, but probably one or two minutes.
>>
>> I guess I have a few questions:
>>
>> 1.) What's going on here? It looks like maybe there's some packet loss
>> and then connection termination gets stuck in a loop because the
>> client timestamp went down?
>> 2.) Is there a better way to mitigate this other than disabling
>> tcp_timestamps or blocking gatech ips?
>> 3.) Is this our problem (ok, obviously our problem since we're
>> affected but...), a kernel problem, or a gatech problem?
>>
>> I'd really appreciate any help on this,
>
> Would you have a pcap file instead ?
>
> It looks a middlebox is broken, I dont think Android could possibly send
> a frame with no payload, but with Push flag.
>
> Neal has some patches that add a rate limiting on DACKS, that we might
> upstream. (per socket rate limiting of 2 DACK per second)
>
> Thanks
>
>
>

^ permalink raw reply

* Re: Invalid timestamp? causing tight ack loop (hundreds of thousands of packets / sec)
From: Avery Fay @ 2015-02-04  8:37 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev, Neal Cardwell
In-Reply-To: <CAO-X30shOhH7tN+msRvbLreeUvZesKARqyU0Lnm3_xoeea9uaQ@mail.gmail.com>

Sorry, forgot to mention: the packet capture has been filtered to only
the ip/port combo from above. I also stopped the capture early before
the traffic would naturally stop because I wanted to disable
timestamps again as soon as possible.

Avery

On Wed, Feb 4, 2015 at 12:35 AM, Avery Fay <avery@mixpanel.com> wrote:
> Sure, https://dl.dropboxusercontent.com/u/9777748/loop.pcap.gz
>
> Also, no idea if it helps, but here's the traceroute:
>
> HOST: apibalancer-wdc-05                          Loss%   Snt   Last
> Avg  Best  Wrst StDev
>   1.|-- 184.173.130.1-static.reverse.softlayer.com   0.0%    10    0.2
>   1.9   0.2   9.6   3.1
>   2.|-- 208.43.118.164-static.reverse.softlayer.com  0.0%    10    0.2
>   0.2   0.2   0.3   0.0
>   3.|-- ae8.bbr02.eq01.wdc02.networklayer.com        0.0%    10    1.2
>   1.3   1.1   2.6   0.5
>   4.|-- ash-b1-link.telia.net                        0.0%    10    1.2
>   4.3   1.1  12.7   5.0
>   5.|-- ash-bb3-link.telia.net                       0.0%    10    1.2
>   2.6   1.1  15.0   4.4
>   6.|-- atl-bb1-link.telia.net                       0.0%    10   13.4
>  13.3 13.3 13.4 0.0
>   7.|-- 213.248.94.220                               0.0%    10   14.2
>  14.2  14.2  14.4   0.1
>   8.|-- 130.207.254.6                                0.0%    10   14.2
>  14.2  14.1  14.3   0.1
>     |  `|-- 130.207.254.185
>   9.|-- gateway2-rtr.gatech.edu                      0.0%    10   14.2
>  14.3 14.1 14.8 0.2
>     |  `|-- 143.215.254.97
>  10.|-- 143.215.254.97                               0.0%    10   14.6
>  14.6  14.4  14.7   0.1
>     |  `|-- 143.215.253.114
>  11.|-- 143.215.253.114                              0.0%    10   15.1
>  14.7  14.5  15.1   0.2
>  12.|-- ???                                         100.0    10    0.0
>   0.0   0.0   0.0   0.0
>
> On Wed, Feb 4, 2015 at 12:03 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>> On Tue, 2015-02-03 at 22:50 -0800, Avery Fay wrote:
>>> Hello,
>>>
>>> Let me say first: if there's a better place to ask this, please point
>>> me in that direction.
>>>
>>> We've been having huge packets / sec spikes in the past few days.
>>> After some investigation, it looks like single connections are getting
>>> stuck in a loop (see tcpdump below). Each "stuck" connection will
>>> generate about 200kpps. It looks like our side is rejecting packets
>>> with "packets rejects in established connections because of timestamp"
>>> from netstat -s (internally PAWSEstab counter) and then generating an
>>> additional packet that we send out. All of these connections originate
>>> from georgia tech, but so far (not completely verified) it doesn't
>>> seem like there's any pattern to the client/os other than the fact
>>> that they're trying to make an https request to us.
>>>
>>> As a temporary countermeasure, we've disabled net.ipv4.tcp_timestamps,
>>> which solves the immediate problem.
>>>
>>> Our server is 174.36.240.86 running Ubuntu 12.04 with kernel 3.13.0-35-generic
>>>
>>> The client is 128.61.57.205 and in this case almost certainly has user
>>> agent (we found successful requests 10 seconds before the tcpdump with
>>> same ip): Dalvik/2.1.0 (Linux; U; Android 5.0; XT1095
>>> Build/LXE22.46-11)
>>>
>>> Beginning of tcpdump:
>>
>> ...
>>
>>>
>>> At this point, it just repeats until some timeout is hit. I haven't
>>> timed it, but probably one or two minutes.
>>>
>>> I guess I have a few questions:
>>>
>>> 1.) What's going on here? It looks like maybe there's some packet loss
>>> and then connection termination gets stuck in a loop because the
>>> client timestamp went down?
>>> 2.) Is there a better way to mitigate this other than disabling
>>> tcp_timestamps or blocking gatech ips?
>>> 3.) Is this our problem (ok, obviously our problem since we're
>>> affected but...), a kernel problem, or a gatech problem?
>>>
>>> I'd really appreciate any help on this,
>>
>> Would you have a pcap file instead ?
>>
>> It looks a middlebox is broken, I dont think Android could possibly send
>> a frame with no payload, but with Push flag.
>>
>> Neal has some patches that add a rate limiting on DACKS, that we might
>> upstream. (per socket rate limiting of 2 DACK per second)
>>
>> Thanks
>>
>>
>>

^ permalink raw reply

* Re: [PATCH v3 15/18] vhost: switch vhost get_indirect() to iov_iter, kill memcpy_fromiovec()
From: Michael S. Tsirkin @ 2015-02-04  8:52 UTC (permalink / raw)
  To: Al Viro; +Cc: David Miller, netdev, kvm, virtualization
In-Reply-To: <1423032009-18367-15-git-send-email-viro@ZenIV.linux.org.uk>

On Wed, Feb 04, 2015 at 06:40:06AM +0000, Al Viro wrote:
> From: Al Viro <viro@zeniv.linux.org.uk>
> 
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: kvm@vger.kernel.org
> Cc: virtualization@lists.linux-foundation.org
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

Acked-by: Michael S. Tsirkin <mst@redhat.com>

> ---
>  drivers/vhost/vhost.c |  6 ++++--
>  include/linux/uio.h   |  1 -
>  lib/iovec.c           | 25 -------------------------
>  3 files changed, 4 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index cb807d0..2ee2826 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -1125,6 +1125,7 @@ static int get_indirect(struct vhost_virtqueue *vq,
>  	struct vring_desc desc;
>  	unsigned int i = 0, count, found = 0;
>  	u32 len = vhost32_to_cpu(vq, indirect->len);
> +	struct iov_iter from;
>  	int ret;
>  
>  	/* Sanity check */
> @@ -1142,6 +1143,7 @@ static int get_indirect(struct vhost_virtqueue *vq,
>  		vq_err(vq, "Translation failure %d in indirect.\n", ret);
>  		return ret;
>  	}
> +	iov_iter_init(&from, READ, vq->indirect, ret, len);
>  
>  	/* We will use the result as an address to read from, so most
>  	 * architectures only need a compiler barrier here. */
> @@ -1164,8 +1166,8 @@ static int get_indirect(struct vhost_virtqueue *vq,
>  			       i, count);
>  			return -EINVAL;
>  		}
> -		if (unlikely(memcpy_fromiovec((unsigned char *)&desc,
> -					      vq->indirect, sizeof desc))) {
> +		if (unlikely(copy_from_iter(&desc, sizeof(desc), &from) !=
> +			     sizeof(desc))) {
>  			vq_err(vq, "Failed indirect descriptor: idx %d, %zx\n",
>  			       i, (size_t)vhost64_to_cpu(vq, indirect->addr) + i * sizeof desc);
>  			return -EINVAL;
> diff --git a/include/linux/uio.h b/include/linux/uio.h
> index 1c5e453..af3439f 100644
> --- a/include/linux/uio.h
> +++ b/include/linux/uio.h
> @@ -135,7 +135,6 @@ static inline void iov_iter_reexpand(struct iov_iter *i, size_t count)
>  size_t csum_and_copy_to_iter(void *addr, size_t bytes, __wsum *csum, struct iov_iter *i);
>  size_t csum_and_copy_from_iter(void *addr, size_t bytes, __wsum *csum, struct iov_iter *i);
>  
> -int memcpy_fromiovec(unsigned char *kdata, struct iovec *iov, int len);
>  int memcpy_fromiovecend(unsigned char *kdata, const struct iovec *iov,
>  			int offset, int len);
>  int memcpy_toiovecend(const struct iovec *v, unsigned char *kdata,
> diff --git a/lib/iovec.c b/lib/iovec.c
> index 2d99cb4..4a90875 100644
> --- a/lib/iovec.c
> +++ b/lib/iovec.c
> @@ -3,31 +3,6 @@
>  #include <linux/uio.h>
>  
>  /*
> - *	Copy iovec to kernel. Returns -EFAULT on error.
> - *
> - *	Note: this modifies the original iovec.
> - */
> -
> -int memcpy_fromiovec(unsigned char *kdata, struct iovec *iov, int len)
> -{
> -	while (len > 0) {
> -		if (iov->iov_len) {
> -			int copy = min_t(unsigned int, len, iov->iov_len);
> -			if (copy_from_user(kdata, iov->iov_base, copy))
> -				return -EFAULT;
> -			len -= copy;
> -			kdata += copy;
> -			iov->iov_base += copy;
> -			iov->iov_len -= copy;
> -		}
> -		iov++;
> -	}
> -
> -	return 0;
> -}
> -EXPORT_SYMBOL(memcpy_fromiovec);
> -
> -/*
>   *	Copy kernel to iovec. Returns -EFAULT on error.
>   */
>  
> -- 
> 2.1.4

^ permalink raw reply

* Re: [PATCH v3 17/18] vhost: don't bother copying iovecs in handle_rx(), kill memcpy_toiovecend()
From: Michael S. Tsirkin @ 2015-02-04  8:52 UTC (permalink / raw)
  To: Al Viro; +Cc: David Miller, netdev, kvm, virtualization
In-Reply-To: <1423032009-18367-17-git-send-email-viro@ZenIV.linux.org.uk>

On Wed, Feb 04, 2015 at 06:40:08AM +0000, Al Viro wrote:
> From: Al Viro <viro@zeniv.linux.org.uk>
> 
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: kvm@vger.kernel.org
> Cc: virtualization@lists.linux-foundation.org
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

Acked-by: Michael S. Tsirkin <mst@redhat.com>

> ---
>  drivers/vhost/net.c | 82 +++++++++++++++--------------------------------------
>  include/linux/uio.h |  3 --
>  lib/iovec.c         | 26 -----------------
>  3 files changed, 23 insertions(+), 88 deletions(-)
> 
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index d86cc9b..e022cc4 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -84,10 +84,6 @@ struct vhost_net_ubuf_ref {
>  
>  struct vhost_net_virtqueue {
>  	struct vhost_virtqueue vq;
> -	/* hdr is used to store the virtio header.
> -	 * Since each iovec has >= 1 byte length, we never need more than
> -	 * header length entries to store the header. */
> -	struct iovec hdr[sizeof(struct virtio_net_hdr_mrg_rxbuf)];
>  	size_t vhost_hlen;
>  	size_t sock_hlen;
>  	/* vhost zerocopy support fields below: */
> @@ -235,44 +231,6 @@ static bool vhost_sock_zcopy(struct socket *sock)
>  		sock_flag(sock->sk, SOCK_ZEROCOPY);
>  }
>  
> -/* Pop first len bytes from iovec. Return number of segments used. */
> -static int move_iovec_hdr(struct iovec *from, struct iovec *to,
> -			  size_t len, int iov_count)
> -{
> -	int seg = 0;
> -	size_t size;
> -
> -	while (len && seg < iov_count) {
> -		size = min(from->iov_len, len);
> -		to->iov_base = from->iov_base;
> -		to->iov_len = size;
> -		from->iov_len -= size;
> -		from->iov_base += size;
> -		len -= size;
> -		++from;
> -		++to;
> -		++seg;
> -	}
> -	return seg;
> -}
> -/* Copy iovec entries for len bytes from iovec. */
> -static void copy_iovec_hdr(const struct iovec *from, struct iovec *to,
> -			   size_t len, int iovcount)
> -{
> -	int seg = 0;
> -	size_t size;
> -
> -	while (len && seg < iovcount) {
> -		size = min(from->iov_len, len);
> -		to->iov_base = from->iov_base;
> -		to->iov_len = size;
> -		len -= size;
> -		++from;
> -		++to;
> -		++seg;
> -	}
> -}
> -
>  /* In case of DMA done not in order in lower device driver for some reason.
>   * upend_idx is used to track end of used idx, done_idx is used to track head
>   * of used idx. Once lower device DMA done contiguously, we will signal KVM
> @@ -570,9 +528,9 @@ static void handle_rx(struct vhost_net *net)
>  		.msg_controllen = 0,
>  		.msg_flags = MSG_DONTWAIT,
>  	};
> -	struct virtio_net_hdr_mrg_rxbuf hdr = {
> -		.hdr.flags = 0,
> -		.hdr.gso_type = VIRTIO_NET_HDR_GSO_NONE
> +	struct virtio_net_hdr hdr = {
> +		.flags = 0,
> +		.gso_type = VIRTIO_NET_HDR_GSO_NONE
>  	};
>  	size_t total_len = 0;
>  	int err, mergeable;
> @@ -580,6 +538,7 @@ static void handle_rx(struct vhost_net *net)
>  	size_t vhost_hlen, sock_hlen;
>  	size_t vhost_len, sock_len;
>  	struct socket *sock;
> +	struct iov_iter fixup;
>  
>  	mutex_lock(&vq->mutex);
>  	sock = vq->private_data;
> @@ -624,14 +583,19 @@ static void handle_rx(struct vhost_net *net)
>  			break;
>  		}
>  		/* We don't need to be notified again. */
> -		if (unlikely((vhost_hlen)))
> -			/* Skip header. TODO: support TSO. */
> -			move_iovec_hdr(vq->iov, nvq->hdr, vhost_hlen, in);
> -		else
> -			/* Copy the header for use in VIRTIO_NET_F_MRG_RXBUF:
> -			 * needed because recvmsg can modify msg_iov. */
> -			copy_iovec_hdr(vq->iov, nvq->hdr, sock_hlen, in);
> -		iov_iter_init(&msg.msg_iter, READ, vq->iov, in, sock_len);
> +		iov_iter_init(&msg.msg_iter, READ, vq->iov, in, vhost_len);
> +		fixup = msg.msg_iter;
> +		if (unlikely((vhost_hlen))) {
> +			/* We will supply the header ourselves
> +			 * TODO: support TSO.
> +			 */
> +			iov_iter_advance(&msg.msg_iter, vhost_hlen);
> +		} else {
> +			/* It'll come from socket; we'll need to patch
> +			 * ->num_buffers over if VIRTIO_NET_F_MRG_RXBUF
> +			 */
> +			iov_iter_advance(&fixup, sizeof(hdr));
> +		}
>  		err = sock->ops->recvmsg(NULL, sock, &msg,
>  					 sock_len, MSG_DONTWAIT | MSG_TRUNC);
>  		/* Userspace might have consumed the packet meanwhile:
> @@ -643,18 +607,18 @@ static void handle_rx(struct vhost_net *net)
>  			vhost_discard_vq_desc(vq, headcount);
>  			continue;
>  		}
> +		/* Supply virtio_net_hdr if VHOST_NET_F_VIRTIO_NET_HDR */
>  		if (unlikely(vhost_hlen) &&
> -		    memcpy_toiovecend(nvq->hdr, (unsigned char *)&hdr, 0,
> -				      vhost_hlen)) {
> +		    copy_to_iter(&hdr, sizeof(hdr), &fixup) != sizeof(hdr)) {
>  			vq_err(vq, "Unable to write vnet_hdr at addr %p\n",
>  			       vq->iov->iov_base);
>  			break;
>  		}
> -		/* TODO: Should check and handle checksum. */
> +		/* Supply (or replace) ->num_buffers if VIRTIO_NET_F_MRG_RXBUF
> +		 * TODO: Should check and handle checksum.
> +		 */
>  		if (likely(mergeable) &&
> -		    memcpy_toiovecend(nvq->hdr, (unsigned char *)&headcount,
> -				      offsetof(typeof(hdr), num_buffers),
> -				      sizeof hdr.num_buffers)) {
> +		    copy_to_iter(&headcount, 2, &fixup) != 2) {
>  			vq_err(vq, "Failed num_buffers write");
>  			vhost_discard_vq_desc(vq, headcount);
>  			break;
> diff --git a/include/linux/uio.h b/include/linux/uio.h
> index af3439f..02bd8a9 100644
> --- a/include/linux/uio.h
> +++ b/include/linux/uio.h
> @@ -137,7 +137,4 @@ size_t csum_and_copy_from_iter(void *addr, size_t bytes, __wsum *csum, struct io
>  
>  int memcpy_fromiovecend(unsigned char *kdata, const struct iovec *iov,
>  			int offset, int len);
> -int memcpy_toiovecend(const struct iovec *v, unsigned char *kdata,
> -		      int offset, int len);
> -
>  #endif
> diff --git a/lib/iovec.c b/lib/iovec.c
> index 4a90875..d8f17a9 100644
> --- a/lib/iovec.c
> +++ b/lib/iovec.c
> @@ -3,32 +3,6 @@
>  #include <linux/uio.h>
>  
>  /*
> - *	Copy kernel to iovec. Returns -EFAULT on error.
> - */
> -
> -int memcpy_toiovecend(const struct iovec *iov, unsigned char *kdata,
> -		      int offset, int len)
> -{
> -	int copy;
> -	for (; len > 0; ++iov) {
> -		/* Skip over the finished iovecs */
> -		if (unlikely(offset >= iov->iov_len)) {
> -			offset -= iov->iov_len;
> -			continue;
> -		}
> -		copy = min_t(unsigned int, iov->iov_len - offset, len);
> -		if (copy_to_user(iov->iov_base + offset, kdata, copy))
> -			return -EFAULT;
> -		offset = 0;
> -		kdata += copy;
> -		len -= copy;
> -	}
> -
> -	return 0;
> -}
> -EXPORT_SYMBOL(memcpy_toiovecend);
> -
> -/*
>   *	Copy iovec to kernel. Returns -EFAULT on error.
>   */
>  
> -- 
> 2.1.4

^ permalink raw reply

* Re: [PATCH v3 17/18] vhost: don't bother copying iovecs in handle_rx(), kill memcpy_toiovecend()
From: Michael S. Tsirkin @ 2015-02-04  9:02 UTC (permalink / raw)
  To: Al Viro; +Cc: netdev, David Miller, kvm, virtualization
In-Reply-To: <1423032009-18367-17-git-send-email-viro@ZenIV.linux.org.uk>

>  		if (likely(mergeable) &&
> -		    memcpy_toiovecend(nvq->hdr, (unsigned char *)&headcount,
> -				      offsetof(typeof(hdr), num_buffers),
> -				      sizeof hdr.num_buffers)) {
> +		    copy_to_iter(&headcount, 2, &fixup) != 2) {
>  			vq_err(vq, "Failed num_buffers write");
>  			vhost_discard_vq_desc(vq, headcount);
>  			break;

BTW this will have to be re-based on top of
http://mid.gmane.org/1422896842-19186-1-git-send-email-mst@redhat.com
taking byte-swap into account.

^ permalink raw reply

* Re: [PATCH v3 18/18] vhost: vhost_scsi_handle_vq() should just use copy_from_user()
From: Michael S. Tsirkin @ 2015-02-04  9:05 UTC (permalink / raw)
  To: Al Viro; +Cc: netdev, virtualization, David Miller, kvm
In-Reply-To: <1423032009-18367-18-git-send-email-viro@ZenIV.linux.org.uk>

On Wed, Feb 04, 2015 at 06:40:09AM +0000, Al Viro wrote:
> From: Al Viro <viro@zeniv.linux.org.uk>
> 
> it has just verified that it asks no more than the length of the
> first segment of iovec.
> 
> And with that the last user of stuff in lib/iovec.c is gone.
> RIP.
> 
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Nicholas A. Bellinger <nab@linux-iscsi.org>
> Cc: kvm@vger.kernel.org
> Cc: virtualization@lists.linux-foundation.org
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

Acked-by: Michael S. Tsirkin <mst@redhat.com>

> ---
>  drivers/vhost/scsi.c |  2 +-
>  include/linux/uio.h  |  2 --
>  lib/Makefile         |  2 +-
>  lib/iovec.c          | 36 ------------------------------------
>  4 files changed, 2 insertions(+), 40 deletions(-)
>  delete mode 100644 lib/iovec.c
> 
> diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
> index d695b16..dc78d87 100644
> --- a/drivers/vhost/scsi.c
> +++ b/drivers/vhost/scsi.c
> @@ -1079,7 +1079,7 @@ vhost_scsi_handle_vq(struct vhost_scsi *vs, struct vhost_virtqueue *vq)
>  			       req_size, vq->iov[0].iov_len);
>  			break;
>  		}
> -		ret = memcpy_fromiovecend(req, &vq->iov[0], 0, req_size);
> +		ret = copy_from_user(req, vq->iov[0].iov_base, req_size);
>  		if (unlikely(ret)) {
>  			vq_err(vq, "Faulted on virtio_scsi_cmd_req\n");
>  			break;
> diff --git a/include/linux/uio.h b/include/linux/uio.h
> index 02bd8a9..3e0cb4e 100644
> --- a/include/linux/uio.h
> +++ b/include/linux/uio.h
> @@ -135,6 +135,4 @@ static inline void iov_iter_reexpand(struct iov_iter *i, size_t count)
>  size_t csum_and_copy_to_iter(void *addr, size_t bytes, __wsum *csum, struct iov_iter *i);
>  size_t csum_and_copy_from_iter(void *addr, size_t bytes, __wsum *csum, struct iov_iter *i);
>  
> -int memcpy_fromiovecend(unsigned char *kdata, const struct iovec *iov,
> -			int offset, int len);
>  #endif
> diff --git a/lib/Makefile b/lib/Makefile
> index 3c3b30b..1071d06 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -24,7 +24,7 @@ obj-y	+= lockref.o
>  
>  obj-y += bcd.o div64.o sort.o parser.o halfmd4.o debug_locks.o random32.o \
>  	 bust_spinlocks.o hexdump.o kasprintf.o bitmap.o scatterlist.o \
> -	 gcd.o lcm.o list_sort.o uuid.o flex_array.o iovec.o clz_ctz.o \
> +	 gcd.o lcm.o list_sort.o uuid.o flex_array.o clz_ctz.o \
>  	 bsearch.o find_last_bit.o find_next_bit.o llist.o memweight.o kfifo.o \
>  	 percpu-refcount.o percpu_ida.o rhashtable.o reciprocal_div.o
>  obj-y += string_helpers.o
> diff --git a/lib/iovec.c b/lib/iovec.c
> deleted file mode 100644
> index d8f17a9..0000000
> --- a/lib/iovec.c
> +++ /dev/null
> @@ -1,36 +0,0 @@
> -#include <linux/uaccess.h>
> -#include <linux/export.h>
> -#include <linux/uio.h>
> -
> -/*
> - *	Copy iovec to kernel. Returns -EFAULT on error.
> - */
> -
> -int memcpy_fromiovecend(unsigned char *kdata, const struct iovec *iov,
> -			int offset, int len)
> -{
> -	/* No data? Done! */
> -	if (len == 0)
> -		return 0;
> -
> -	/* Skip over the finished iovecs */
> -	while (offset >= iov->iov_len) {
> -		offset -= iov->iov_len;
> -		iov++;
> -	}
> -
> -	while (len > 0) {
> -		u8 __user *base = iov->iov_base + offset;
> -		int copy = min_t(unsigned int, len, iov->iov_len - offset);
> -
> -		offset = 0;
> -		if (copy_from_user(kdata, base, copy))
> -			return -EFAULT;
> -		len -= copy;
> -		kdata += copy;
> -		iov++;
> -	}
> -
> -	return 0;
> -}
> -EXPORT_SYMBOL(memcpy_fromiovecend);
> -- 
> 2.1.4

^ permalink raw reply

* Re: [PATCH v3 16/18] vhost: don't bother with copying iovec in handle_tx()
From: Michael S. Tsirkin @ 2015-02-04  9:13 UTC (permalink / raw)
  To: Al Viro; +Cc: netdev, David Miller, kvm, virtualization
In-Reply-To: <1423032009-18367-16-git-send-email-viro@ZenIV.linux.org.uk>

On Wed, Feb 04, 2015 at 06:40:07AM +0000, Al Viro wrote:
> From: Al Viro <viro@zeniv.linux.org.uk>
> 
> just advance the msg.msg_iter and be done with that.
> 
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: kvm@vger.kernel.org
> Cc: virtualization@lists.linux-foundation.org
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

Acked-by: Michael S. Tsirkin <mst@redhat.com>

> ---
>  drivers/vhost/net.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 6906f76..d86cc9b 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -336,7 +336,7 @@ static void handle_tx(struct vhost_net *net)
>  {
>  	struct vhost_net_virtqueue *nvq = &net->vqs[VHOST_NET_VQ_TX];
>  	struct vhost_virtqueue *vq = &nvq->vq;
> -	unsigned out, in, s;
> +	unsigned out, in;
>  	int head;
>  	struct msghdr msg = {
>  		.msg_name = NULL,
> @@ -395,16 +395,17 @@ static void handle_tx(struct vhost_net *net)
>  			break;
>  		}
>  		/* Skip header. TODO: support TSO. */
> -		s = move_iovec_hdr(vq->iov, nvq->hdr, hdr_size, out);
>  		len = iov_length(vq->iov, out);
>  		iov_iter_init(&msg.msg_iter, WRITE, vq->iov, out, len);
> +		iov_iter_advance(&msg.msg_iter, hdr_size);
>  		/* Sanity check */
> -		if (!len) {
> +		if (!iov_iter_count(&msg.msg_iter)) {
>  			vq_err(vq, "Unexpected header len for TX: "
>  			       "%zd expected %zd\n",
> -			       iov_length(nvq->hdr, s), hdr_size);
> +			       len, hdr_size);
>  			break;
>  		}
> +		len = iov_iter_count(&msg.msg_iter);
>  
>  		zcopy_used = zcopy && len >= VHOST_GOODCOPY_LEN
>  				   && (nvq->upend_idx + 1) % UIO_MAXIOV !=
> -- 
> 2.1.4

^ permalink raw reply

* [PATCH 1/3] bridge: Base the BR_PROXYARP decision on the target port flag
From: Jouni Malinen @ 2015-02-04  9:51 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Kyeyoon Park, Jouni Malinen

From: Kyeyoon Park <kyeyoonp@codeaurora.org>

When doing proxy ARP, instead of checking the bridge port flag for
BR_PROXYARP on the bridge port on which the frame was received, check
the bridge port flag of the bridge port to which the target device
belongs.

Signed-off-by: Kyeyoon Park <kyeyoonp@codeaurora.org>
Signed-off-by: Jouni Malinen <jouni@codeaurora.org>
---
 net/bridge/br_input.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index e2aa7be..cfd46c6 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -105,7 +105,7 @@ static void br_do_proxy_arp(struct sk_buff *skb, struct net_bridge *br,
 		}
 
 		f = __br_fdb_get(br, n->ha, vid);
-		if (f)
+		if (f && f->dst->flags & BR_PROXYARP)
 			arp_send(ARPOP_REPLY, ETH_P_ARP, sip, skb->dev, tip,
 				 sha, n->ha, sha);
 
@@ -155,7 +155,6 @@ int br_handle_frame_finish(struct sk_buff *skb)
 
 	if (is_broadcast_ether_addr(dest)) {
 		if (IS_ENABLED(CONFIG_INET) &&
-		    p->flags & BR_PROXYARP &&
 		    skb->protocol == htons(ETH_P_ARP))
 			br_do_proxy_arp(skb, br, vid);
 
-- 
1.9.1

^ permalink raw reply related

* [PATCH 2/3] bridge: Selectively prevent bridge port flooding for proxy ARP
From: Jouni Malinen @ 2015-02-04  9:51 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Kyeyoon Park, Jouni Malinen
In-Reply-To: <1423043467-11148-1-git-send-email-jouni@codeaurora.org>

From: Kyeyoon Park <kyeyoonp@codeaurora.org>

Rather than completely blocking the bridge port flooding when
BR_PROXYARP is enabled, selectively prevent bridge port flooding for the
proxy ARP relevant frames. This is done by marking the "skb" with this
info when proxy ARP code executes.

Signed-off-by: Kyeyoon Park <kyeyoonp@codeaurora.org>
Signed-off-by: Jouni Malinen <jouni@codeaurora.org>
---
 net/bridge/br_forward.c | 3 ++-
 net/bridge/br_input.c   | 6 +++++-
 net/bridge/br_private.h | 1 +
 3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c
index f96933a..81b4bed 100644
--- a/net/bridge/br_forward.c
+++ b/net/bridge/br_forward.c
@@ -186,7 +186,8 @@ static void br_flood(struct net_bridge *br, struct sk_buff *skb,
 			continue;
 
 		/* Do not flood to ports that enable proxy ARP */
-		if (p->flags & BR_PROXYARP)
+		if (p->flags & BR_PROXYARP &&
+		    BR_INPUT_SKB_CB(skb)->proxyarp_replied)
 			continue;
 
 		prev = maybe_deliver(prev, p, skb, __packet_hook);
diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index cfd46c6..41d1d3f 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -68,6 +68,8 @@ static void br_do_proxy_arp(struct sk_buff *skb, struct net_bridge *br,
 	u8 *arpptr, *sha;
 	__be32 sip, tip;
 
+	BR_INPUT_SKB_CB(skb)->proxyarp_replied = false;
+
 	if (dev->flags & IFF_NOARP)
 		return;
 
@@ -105,9 +107,11 @@ static void br_do_proxy_arp(struct sk_buff *skb, struct net_bridge *br,
 		}
 
 		f = __br_fdb_get(br, n->ha, vid);
-		if (f && f->dst->flags & BR_PROXYARP)
+		if (f && f->dst->flags & BR_PROXYARP) {
 			arp_send(ARPOP_REPLY, ETH_P_ARP, sip, skb->dev, tip,
 				 sha, n->ha, sha);
+			BR_INPUT_SKB_CB(skb)->proxyarp_replied = true;
+		}
 
 		neigh_release(n);
 	}
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index de09199..c32e279 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -305,6 +305,7 @@ struct br_input_skb_cb {
 #endif
 
 	u16 frag_max_size;
+	bool proxyarp_replied;
 
 #ifdef CONFIG_BRIDGE_VLAN_FILTERING
 	bool vlan_filtered;
-- 
1.9.1

^ permalink raw reply related

* [PATCH 3/3] bridge: Allow proxy ARP for unicast ARP requests
From: Jouni Malinen @ 2015-02-04  9:51 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Kyeyoon Park, Jouni Malinen
In-Reply-To: <1423043467-11148-1-git-send-email-jouni@codeaurora.org>

From: Kyeyoon Park <kyeyoonp@codeaurora.org>

It is possible for unicast ARP requests to be used and the same proxyarp
mechanism should work for them as well as for the broadcast case.

Signed-off-by: Kyeyoon Park <kyeyoonp@codeaurora.org>
Signed-off-by: Jouni Malinen <jouni@codeaurora.org>
---
 net/bridge/br_input.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index 41d1d3f..c60100f 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -157,11 +157,11 @@ int br_handle_frame_finish(struct sk_buff *skb)
 
 	dst = NULL;
 
-	if (is_broadcast_ether_addr(dest)) {
-		if (IS_ENABLED(CONFIG_INET) &&
-		    skb->protocol == htons(ETH_P_ARP))
-			br_do_proxy_arp(skb, br, vid);
+	if (IS_ENABLED(CONFIG_INET) &&
+	    skb->protocol == htons(ETH_P_ARP))
+		br_do_proxy_arp(skb, br, vid);
 
+	if (is_broadcast_ether_addr(dest)) {
 		skb2 = skb;
 		unicast = false;
 	} else if (is_multicast_ether_addr(dest)) {
-- 
1.9.1

^ permalink raw reply related

* Re: [PATCH] net: sched: One function call less in em_meta_change() after error detection
From: SF Markus Elfring @ 2015-02-04  9:54 UTC (permalink / raw)
  To: David Miller
  Cc: Jamal Hadi Salim, netdev, linux-kernel, kernel-janitors,
	Julia Lawall
In-Reply-To: <20150203.161054.379086029122558267.davem@davemloft.net>

>> The meta_delete() function could be called in four cases by the
>> em_meta_change() function during error handling even if the passed
>> variable "meta" contained still a null pointer.
>>
>> * This implementation detail could be improved by adjustments for jump labels.
>>
>> * Let us return immediately after the first failed function call according to
>>   the current Linux coding style convention.
>>
>> * Let us delete also unnecessary checks for the variables "err" and
>>   "meta" there.
> 
> I kind of like the way the code is now, branching to the end of the function
> even when cleanups are not necessary.

I would appreciate if the affected exception handling can become also
a bit more efficient.

Regards,
Markus


^ permalink raw reply

* [PATCHv2 net-next] cxgb4: Add low latency socket busy_poll support
From: Hariprasad Shenai @ 2015-02-04 10:02 UTC (permalink / raw)
  To: netdev; +Cc: davem, leedom, anish, nirranjan, kumaras, Hariprasad Shenai

cxgb_busy_poll, corresponding to ndo_busy_poll, gets called by the socket
waiting for data.

With busy_poll enabled, improvement is seen in latency numbers as observed by
collecting netperf TCP_RR numbers.
Below are latency number, with and without busy-poll, in a switched environment
for a particular msg size:
netperf command: netperf -4 -H <ip> -l 30 -t TCP_RR -- -r1,1
Latency without busy-poll: ~16.25 us
Latency with busy-poll   : ~08.79 us

Based on original work by Kumar Sanghvi <kumaras@chelsio.com>

Signed-off-by: Hariprasad Shenai <hariprasad@chelsio.com>
---
V2: fix napi poll return value for repoll, thanks to pointers by
    Eric Dumazet <eric.dumazet@gmail.com>

 drivers/net/ethernet/chelsio/cxgb4/cxgb4.h      |  113 +++++++++++++++++++++++
 drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c |   16 +++-
 drivers/net/ethernet/chelsio/cxgb4/sge.c        |   47 +++++++++-
 drivers/net/ethernet/chelsio/cxgb4/t4_values.h  |    1 +
 4 files changed, 174 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h b/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
index fb6980a..55019c9 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
@@ -476,6 +476,22 @@ struct sge_rspq {                   /* state for an SGE response queue */
 	struct adapter *adap;
 	struct net_device *netdev;  /* associated net device */
 	rspq_handler_t handler;
+#ifdef CONFIG_NET_RX_BUSY_POLL
+#define CXGB_POLL_STATE_IDLE		0
+#define CXGB_POLL_STATE_NAPI		BIT(0) /* NAPI owns this poll */
+#define CXGB_POLL_STATE_POLL		BIT(1) /* poll owns this poll */
+#define CXGB_POLL_STATE_NAPI_YIELD	BIT(2) /* NAPI yielded this poll */
+#define CXGB_POLL_STATE_POLL_YIELD	BIT(3) /* poll yielded this poll */
+#define CXGB_POLL_YIELD			(CXGB_POLL_STATE_NAPI_YIELD |   \
+					 CXGB_POLL_STATE_POLL_YIELD)
+#define CXGB_POLL_LOCKED		(CXGB_POLL_STATE_NAPI |         \
+					 CXGB_POLL_STATE_POLL)
+#define CXGB_POLL_USER_PEND		(CXGB_POLL_STATE_POLL |         \
+					 CXGB_POLL_STATE_POLL_YIELD)
+	unsigned int bpoll_state;
+	spinlock_t bpoll_lock;		/* lock for busy poll */
+#endif /* CONFIG_NET_RX_BUSY_POLL */
+
 };
 
 struct sge_eth_stats {              /* Ethernet queue statistics */
@@ -880,6 +896,102 @@ static inline struct adapter *netdev2adap(const struct net_device *dev)
 	return netdev2pinfo(dev)->adapter;
 }
 
+#ifdef CONFIG_NET_RX_BUSY_POLL
+static inline void cxgb_busy_poll_init_lock(struct sge_rspq *q)
+{
+	spin_lock_init(&q->bpoll_lock);
+	q->bpoll_state = CXGB_POLL_STATE_IDLE;
+}
+
+static inline bool cxgb_poll_lock_napi(struct sge_rspq *q)
+{
+	bool rc = true;
+
+	spin_lock(&q->bpoll_lock);
+	if (q->bpoll_state & CXGB_POLL_LOCKED) {
+		q->bpoll_state |= CXGB_POLL_STATE_NAPI_YIELD;
+		rc = false;
+	} else {
+		q->bpoll_state = CXGB_POLL_STATE_NAPI;
+	}
+	spin_unlock(&q->bpoll_lock);
+	return rc;
+}
+
+static inline bool cxgb_poll_unlock_napi(struct sge_rspq *q)
+{
+	bool rc = false;
+
+	spin_lock(&q->bpoll_lock);
+	if (q->bpoll_state & CXGB_POLL_STATE_POLL_YIELD)
+		rc = true;
+	q->bpoll_state = CXGB_POLL_STATE_IDLE;
+	spin_unlock(&q->bpoll_lock);
+	return rc;
+}
+
+static inline bool cxgb_poll_lock_poll(struct sge_rspq *q)
+{
+	bool rc = true;
+
+	spin_lock_bh(&q->bpoll_lock);
+	if (q->bpoll_state & CXGB_POLL_LOCKED) {
+		q->bpoll_state |= CXGB_POLL_STATE_POLL_YIELD;
+		rc = false;
+	} else {
+		q->bpoll_state |= CXGB_POLL_STATE_POLL;
+	}
+	spin_unlock_bh(&q->bpoll_lock);
+	return rc;
+}
+
+static inline bool cxgb_poll_unlock_poll(struct sge_rspq *q)
+{
+	bool rc = false;
+
+	spin_lock_bh(&q->bpoll_lock);
+	if (q->bpoll_state & CXGB_POLL_STATE_POLL_YIELD)
+		rc = true;
+	q->bpoll_state = CXGB_POLL_STATE_IDLE;
+	spin_unlock_bh(&q->bpoll_lock);
+	return rc;
+}
+
+static inline bool cxgb_poll_busy_polling(struct sge_rspq *q)
+{
+	return q->bpoll_state & CXGB_POLL_USER_PEND;
+}
+#else
+static inline void cxgb_busy_poll_init_lock(struct sge_rspq *q)
+{
+}
+
+static inline bool cxgb_poll_lock_napi(struct sge_rspq *q)
+{
+	return true;
+}
+
+static inline bool cxgb_poll_unlock_napi(struct sge_rspq *q)
+{
+	return false;
+}
+
+static inline bool cxgb_poll_lock_poll(struct sge_rspq *q)
+{
+	return false;
+}
+
+static inline bool cxgb_poll_unlock_poll(struct sge_rspq *q)
+{
+	return false;
+}
+
+static inline bool cxgb_poll_busy_polling(struct sge_rspq *q)
+{
+	return false;
+}
+#endif /* CONFIG_NET_RX_BUSY_POLL */
+
 void t4_os_portmod_changed(const struct adapter *adap, int port_id);
 void t4_os_link_changed(struct adapter *adap, int port_id, int link_stat);
 
@@ -908,6 +1020,7 @@ irqreturn_t t4_sge_intr_msix(int irq, void *cookie);
 int t4_sge_init(struct adapter *adap);
 void t4_sge_start(struct adapter *adap);
 void t4_sge_stop(struct adapter *adap);
+int cxgb_busy_poll(struct napi_struct *napi);
 extern int dbfifo_int_thresh;
 
 #define for_each_port(adapter, iter) \
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
index 5bf490a..041742b 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
@@ -923,8 +923,14 @@ static void quiesce_rx(struct adapter *adap)
 	for (i = 0; i < ARRAY_SIZE(adap->sge.ingr_map); i++) {
 		struct sge_rspq *q = adap->sge.ingr_map[i];
 
-		if (q && q->handler)
+		if (q && q->handler) {
 			napi_disable(&q->napi);
+			local_bh_disable();
+			while (!cxgb_poll_lock_napi(q))
+				mdelay(1);
+			local_bh_enable();
+		}
+
 	}
 }
 
@@ -940,8 +946,10 @@ static void enable_rx(struct adapter *adap)
 
 		if (!q)
 			continue;
-		if (q->handler)
+		if (q->handler) {
+			cxgb_busy_poll_init_lock(q);
 			napi_enable(&q->napi);
+		}
 		/* 0-increment GTS to start the timer and enable interrupts */
 		t4_write_reg(adap, MYPF_REG(SGE_PF_GTS_A),
 			     SEINTARM_V(q->intr_params) |
@@ -4563,6 +4571,10 @@ static const struct net_device_ops cxgb4_netdev_ops = {
 #ifdef CONFIG_NET_POLL_CONTROLLER
 	.ndo_poll_controller  = cxgb_netpoll,
 #endif
+#ifdef CONFIG_NET_RX_BUSY_POLL
+	.ndo_busy_poll        = cxgb_busy_poll,
+#endif
+
 };
 
 void t4_fatal_err(struct adapter *adap)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/sge.c b/drivers/net/ethernet/chelsio/cxgb4/sge.c
index 6191561..b4b9f60 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/sge.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/sge.c
@@ -43,6 +43,9 @@
 #include <linux/export.h>
 #include <net/ipv6.h>
 #include <net/tcp.h>
+#ifdef CONFIG_NET_RX_BUSY_POLL
+#include <net/busy_poll.h>
+#endif /* CONFIG_NET_RX_BUSY_POLL */
 #include "cxgb4.h"
 #include "t4_regs.h"
 #include "t4_values.h"
@@ -1720,6 +1723,7 @@ static void do_gro(struct sge_eth_rxq *rxq, const struct pkt_gl *gl,
 	skb->truesize += skb->data_len;
 	skb->ip_summed = CHECKSUM_UNNECESSARY;
 	skb_record_rx_queue(skb, rxq->rspq.idx);
+	skb_mark_napi_id(skb, &rxq->rspq.napi);
 	if (rxq->rspq.netdev->features & NETIF_F_RXHASH)
 		skb_set_hash(skb, (__force u32)pkt->rsshdr.hash_val,
 			     PKT_HASH_TYPE_L3);
@@ -1763,6 +1767,7 @@ int t4_ethrx_handler(struct sge_rspq *q, const __be64 *rsp,
 	csum_ok = pkt->csum_calc && !pkt->err_vec &&
 		  (q->netdev->features & NETIF_F_RXCSUM);
 	if ((pkt->l2info & htonl(RXF_TCP_F)) &&
+	    !(cxgb_poll_busy_polling(q)) &&
 	    (q->netdev->features & NETIF_F_GRO) && csum_ok && !pkt->ip_frag) {
 		do_gro(rxq, si, pkt);
 		return 0;
@@ -1801,6 +1806,7 @@ int t4_ethrx_handler(struct sge_rspq *q, const __be64 *rsp,
 		__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), ntohs(pkt->vlan));
 		rxq->stats.vlan_ex++;
 	}
+	skb_mark_napi_id(skb, &q->napi);
 	netif_receive_skb(skb);
 	return 0;
 }
@@ -1963,6 +1969,38 @@ static int process_responses(struct sge_rspq *q, int budget)
 	return budget - budget_left;
 }
 
+#ifdef CONFIG_NET_RX_BUSY_POLL
+int cxgb_busy_poll(struct napi_struct *napi)
+{
+	struct sge_rspq *q = container_of(napi, struct sge_rspq, napi);
+	unsigned int params, work_done;
+	u32 val;
+
+	if (!cxgb_poll_lock_poll(q))
+		return LL_FLUSH_BUSY;
+
+	work_done = process_responses(q, 4);
+	params = QINTR_TIMER_IDX(TIMERREG_COUNTER0_X) | QINTR_CNT_EN;
+	q->next_intr_params = params;
+	val = CIDXINC_V(work_done) | SEINTARM_V(params);
+
+	/* If we don't have access to the new User GTS (T5+), use the old
+	 * doorbell mechanism; otherwise use the new BAR2 mechanism.
+	 */
+	if (unlikely(!q->bar2_addr))
+		t4_write_reg(q->adap, MYPF_REG(SGE_PF_GTS_A),
+			     val | INGRESSQID_V((u32)q->cntxt_id));
+	else {
+		writel(val | INGRESSQID_V(q->bar2_qid),
+		       q->bar2_addr + SGE_UDB_GTS);
+		wmb();
+	}
+
+	cxgb_poll_unlock_poll(q);
+	return work_done;
+}
+#endif /* CONFIG_NET_RX_BUSY_POLL */
+
 /**
  *	napi_rx_handler - the NAPI handler for Rx processing
  *	@napi: the napi instance
@@ -1978,9 +2016,13 @@ static int napi_rx_handler(struct napi_struct *napi, int budget)
 {
 	unsigned int params;
 	struct sge_rspq *q = container_of(napi, struct sge_rspq, napi);
-	int work_done = process_responses(q, budget);
+	int work_done;
 	u32 val;
 
+	if (!cxgb_poll_lock_napi(q))
+		return budget;
+
+	work_done = process_responses(q, budget);
 	if (likely(work_done < budget)) {
 		int timer_index;
 
@@ -2018,6 +2060,7 @@ static int napi_rx_handler(struct napi_struct *napi, int budget)
 		       q->bar2_addr + SGE_UDB_GTS);
 		wmb();
 	}
+	cxgb_poll_unlock_napi(q);
 	return work_done;
 }
 
@@ -2341,6 +2384,7 @@ int t4_sge_alloc_rxq(struct adapter *adap, struct sge_rspq *iq, bool fwevtq,
 		goto err;
 
 	netif_napi_add(dev, &iq->napi, napi_rx_handler, 64);
+	napi_hash_add(&iq->napi);
 	iq->cur_desc = iq->desc;
 	iq->cidx = 0;
 	iq->gen = 1;
@@ -2598,6 +2642,7 @@ static void free_rspq_fl(struct adapter *adap, struct sge_rspq *rq,
 		   rq->cntxt_id, fl_id, 0xffff);
 	dma_free_coherent(adap->pdev_dev, (rq->size + 1) * rq->iqe_len,
 			  rq->desc, rq->phys_addr);
+	napi_hash_del(&rq->napi);
 	netif_napi_del(&rq->napi);
 	rq->netdev = NULL;
 	rq->cntxt_id = rq->abs_id = 0;
diff --git a/drivers/net/ethernet/chelsio/cxgb4/t4_values.h b/drivers/net/ethernet/chelsio/cxgb4/t4_values.h
index a404844..997ec87 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/t4_values.h
+++ b/drivers/net/ethernet/chelsio/cxgb4/t4_values.h
@@ -59,6 +59,7 @@
 
 /* GTS register */
 #define SGE_TIMERREGS			6
+#define TIMERREG_COUNTER0_X		0
 
 /* T5 and later support a new BAR2-based doorbell mechanism for Egress Queues.
  * The User Doorbells are each 128 bytes in length with a Simple Doorbell at
-- 
1.7.1

^ permalink raw reply related

* Re: PROBLEM: Bnx2x Checksum/Length Error Over GRE Tunnel
From: Eren Türkay @ 2015-02-04 10:00 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Network Development
In-Reply-To: <1422989196.907.34.camel@edumazet-glaptop2.roam.corp.google.com>

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

On 03-02-2015 20:46, Eric Dumazet wrote:
> Can you please send : ifconfig -a, as I suspect an MTU issue

You are right. Previously I set gre interface MTU to 1500 and NIC interface had 9000 MTU.
I was able to get 3Gbit in a single flow, and 5-6Gbit in parallel. When I recreated the interfaces,
the MTU of gre was set 8976 and I can get 9.5Gbit in a single flow. The reason why I set 1500 on
gre interface was to mimic openvswitch setup as the main problem I have been investigating is VM
network performance problem (they have 3Gbit throughput). I have OpenStack installation using OVS with GRE
and those interfaces are set 1500 MTU by default. I tried to test iproute2 implementation with 1500.

With correct MTU setting in gre interface, it seems to work without checksumming and offloading
but I haven't tested my original OVS setting. I will tweak OVS bridge MTU settings and try to get
it work. If baremetal-to-baremetal works, I suspect there is an issue with OVS setup.
However, when I enable tx on, netperf cannot even connect.

"ifconfig -a" and "ovs-vsctl show" result is attached.

I am not sure if tx offloading will affect the VM performance. Regardless of OVS problem, there seems to be a
bug with segmentation offloading as it's only possible to use the interface with "tx off". I have compiled the
kernel with debugging on and I can enable debugging on related parts in the kernel for further information.

> Here I easily reach line rate with a single flow.
> 
> ip tunnel add gre1 mode gre remote 10.246.11.52 local 10.246.11.51
> ip link set gre1 up
> ip addr add 7.7.8.51/24 dev gre1

ip tunnel add gre1 mode gre remote 10.20.0.162 local 10.20.0.161
ip l set gre1 up
ip addr add 7.7.8.161/24 dev gre1

root@compute1vx:~# ip r get 7.7.8.162
7.7.8.162 dev gre1  src 7.7.8.161 
    cache  expires 539sec mtu 8976


root@compute1vx:~# ethtool -K em1 tx off
Actual changes:
tx-checksumming: off
        tx-checksum-ipv4: off
        tx-checksum-ipv6: off
tcp-segmentation-offload: off
        tx-tcp-segmentation: off [requested on]
        tx-tcp-ecn-segmentation: off [requested on]
        tx-tcp6-segmentation: off [requested on]

root@compute1vx:~# netperf -H 7.7.8.162 -Cc                                                                                                                                                                                                   
MIGRATED TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 7.7.8.162 () port 0 AF_INET : demo
Recv   Send    Send                          Utilization       Service Demand
Socket Socket  Message  Elapsed              Send     Recv     Send    Recv
Size   Size    Size     Time     Throughput  local    remote   local   remote
bytes  bytes   bytes    secs.    10^6bits/s  % S      % S      us/KB   us/KB

 87380  16384  16384    10.00      9534.00   4.05     4.39     0.835   0.906  


root@compute1vx:~# ethtool -K em1 tx on
Actual changes:
tx-checksumming: on
        tx-checksum-ipv4: on
        tx-checksum-ipv6: on
tcp-segmentation-offload: on
        tx-tcp-segmentation: on
        tx-tcp-ecn-segmentation: on
        tx-tcp6-segmentation: on

root@compute1vx:~# netperf -H 7.7.8.162 -Cc
establish control: are you sure there is a netserver listening on 7.7.8.162 at port 12865?
establish_control could not establish the control connection from 0.0.0.0 port 0 address family AF_UNSPEC to 7.7.8.162 port 12865 address family AF_INET

Regards,
Eren

-- 
System Administrator
https://skyatlas.com/

[-- Attachment #2: ifconfig-and-ovs-result.txt --]
[-- Type: text/plain, Size: 6306 bytes --]

root@compute1vx:~# ifconfig -a
br-int    Link encap:Ethernet  HWaddr 62:6e:a4:45:76:4d  
          inet6 addr: fe80::e897:f0ff:fee3:5a0d/64 Scope:Link
          UP BROADCAST RUNNING  MTU:1500  Metric:1
          RX packets:50 errors:0 dropped:0 overruns:0 frame:0
          TX packets:8 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:0 
          RX bytes:3856 (3.8 KB)  TX bytes:648 (648.0 B)

br-tun    Link encap:Ethernet  HWaddr 72:52:f7:60:bc:4a  
          inet6 addr: fe80::70d8:27ff:fe57:36fd/64 Scope:Link
          UP BROADCAST RUNNING  MTU:1500  Metric:1
          RX packets:0 errors:0 dropped:0 overruns:0 frame:0
          TX packets:8 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:0 
          RX bytes:0 (0.0 B)  TX bytes:648 (648.0 B)

em1       Link encap:Ethernet  HWaddr 00:26:55:1f:9d:a0  
          inet addr:10.20.0.161  Bcast:10.20.0.255  Mask:255.255.255.0
          inet6 addr: fe80::226:55ff:fe1f:9da0/64 Scope:Link
          UP BROADCAST RUNNING MULTICAST  MTU:9000  Metric:1
          RX packets:2775967 errors:1204 dropped:0 overruns:1204 frame:0
          TX packets:19544604 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:1000 
          RX bytes:309879554 (309.8 MB)  TX bytes:80709251505 (80.7 GB)
          Interrupt:42 Memory:fb000000-fb7fffff 

em2       Link encap:Ethernet  HWaddr 00:26:55:1f:9d:a4  
          inet addr:192.168.88.61  Bcast:192.168.88.255  Mask:255.255.255.0
          inet6 addr: fe80::226:55ff:fe1f:9da4/64 Scope:Link
          UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
          RX packets:981996 errors:0 dropped:0 overruns:0 frame:0
          TX packets:504463 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:1000 
          RX bytes:1423897061 (1.4 GB)  TX bytes:38829176 (38.8 MB)
          Interrupt:53 Memory:fa000000-fa7fffff 

gre0      Link encap:UNSPEC  HWaddr 00-00-00-00-32-36-3A-35-00-00-00-00-00-00-00-00  
          NOARP  MTU:1476  Metric:1
          RX packets:0 errors:0 dropped:0 overruns:0 frame:0
          TX packets:0 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:0 
          RX bytes:0 (0.0 B)  TX bytes:0 (0.0 B)

gre1      Link encap:UNSPEC  HWaddr 0A-14-00-A1-00-00-00-00-00-00-00-00-00-00-00-00  
          inet addr:7.7.8.161  P-t-P:7.7.8.161  Mask:255.255.255.0
          inet6 addr: fe80::5efe:a14:a1/64 Scope:Link
          UP POINTOPOINT RUNNING NOARP  MTU:8976  Metric:1
          RX packets:2541984 errors:0 dropped:0 overruns:0 frame:0
          TX packets:1704256 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:0 
          RX bytes:133877496 (133.8 MB)  TX bytes:78944172512 (78.9 GB)

gretap0   Link encap:Ethernet  HWaddr 00:00:00:00:00:00  
          BROADCAST MULTICAST  MTU:1462  Metric:1
          RX packets:0 errors:0 dropped:0 overruns:0 frame:0
          TX packets:0 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:1000 
          RX bytes:0 (0.0 B)  TX bytes:0 (0.0 B)

lo        Link encap:Local Loopback  
          inet addr:127.0.0.1  Mask:255.0.0.0
          inet6 addr: ::1/128 Scope:Host
          UP LOOPBACK RUNNING  MTU:65536  Metric:1
          RX packets:15 errors:0 dropped:0 overruns:0 frame:0
          TX packets:15 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:0 
          RX bytes:1878 (1.8 KB)  TX bytes:1878 (1.8 KB)

ovs-system Link encap:Ethernet  HWaddr fa:69:1a:53:4f:73  
          BROADCAST MULTICAST  MTU:1500  Metric:1
          RX packets:0 errors:0 dropped:0 overruns:0 frame:0
          TX packets:0 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:0 
          RX bytes:0 (0.0 B)  TX bytes:0 (0.0 B)

qbre90aa6bf-36 Link encap:Ethernet  HWaddr 3a:d6:2c:ba:09:57  
          inet6 addr: fe80::387c:4bff:fe67:904f/64 Scope:Link
          UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
          RX packets:44 errors:0 dropped:0 overruns:0 frame:0
          TX packets:8 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:0 
          RX bytes:2760 (2.7 KB)  TX bytes:648 (648.0 B)

qvbe90aa6bf-36 Link encap:Ethernet  HWaddr 3a:d6:2c:ba:09:57  
          inet6 addr: fe80::38d6:2cff:feba:957/64 Scope:Link
          UP BROADCAST RUNNING PROMISC MULTICAST  MTU:1500  Metric:1
          RX packets:45 errors:0 dropped:0 overruns:0 frame:0
          TX packets:15 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:1000 
          RX bytes:3466 (3.4 KB)  TX bytes:1206 (1.2 KB)

qvoe90aa6bf-36 Link encap:Ethernet  HWaddr d2:d7:77:9a:d9:1e  
          inet6 addr: fe80::d0d7:77ff:fe9a:d91e/64 Scope:Link
          UP BROADCAST RUNNING PROMISC MULTICAST  MTU:1500  Metric:1
          RX packets:15 errors:0 dropped:0 overruns:0 frame:0
          TX packets:45 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:1000 
          RX bytes:1206 (1.2 KB)  TX bytes:3466 (3.4 KB)

virbr0    Link encap:Ethernet  HWaddr 8e:54:70:cb:8a:fc  
          inet addr:192.168.122.1  Bcast:192.168.122.255  Mask:255.255.255.0
          UP BROADCAST MULTICAST  MTU:1500  Metric:1
          RX packets:0 errors:0 dropped:0 overruns:0 frame:0
          TX packets:0 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:0 
          RX bytes:0 (0.0 B)  TX bytes:0 (0.0 B)


root@compute1vx:~# ovs-vsctl show
db3856c4-3af5-45af-9676-debc379a3e9a
    Bridge br-int
        fail_mode: secure
        Port patch-tun
            Interface patch-tun
                type: patch
                options: {peer=patch-int}
        Port br-int
            Interface br-int
                type: internal
        Port "qvoe90aa6bf-36"
            tag: 1
            Interface "qvoe90aa6bf-36"
    Bridge br-tun
        Port br-tun
            Interface br-tun
                type: internal
        Port patch-int
            Interface patch-int
                type: patch
                options: {peer=patch-tun}
        Port "gre-0a1400a2"
            Interface "gre-0a1400a2"
                type: gre
                options: {df_default="true", in_key=flow, local_ip="10.20.0.161", out_key=flow, remote_ip="10.20.0.162"}
    ovs_version: "2.0.2"


^ permalink raw reply

* Re: [PATCH net v3] net: ipv6: allow explicitly choosing optimistic addresses
From: Erik Kline @ 2015-02-04 10:38 UTC (permalink / raw)
  To: Lorenzo Colitti; +Cc: netdev@vger.kernel.org, Hannes Frederic Sowa
In-Reply-To: <CAKD1Yr0k44kwwtfWN-t9C1f7WKT0o=yimPwbfLtBnnvUvXrm=w@mail.gmail.com>

>> +               bool optimistic_ok = (ifp->flags & IFA_F_OPTIMISTIC) &&
>> +                                    (banned_flags == IFA_F_TENTATIVE);
>
> Not sure if this can happen in any real use case, but I think that
> technically this is incorrect if banned_flags contains both
> IFA_F_TENTATIVE and other flags that aren't IFA_F_OPTIMISTIC. For
> example, suppose banned_flags = IFA_F_TENTATIVE | IFA_F_PERMANENT. In
> that case, I think the code would reject an address with
> IFA_F_TENTATIVE | IFA_F_OPTIMISTIC. You might be able to fix that

Exactly what I was trying to address, but I tried to choose the
uber-conservative option and require folks to explicitly list when
optimistic should be treated distinctly from tentative.

That of course, wasn't really future-proof and someone might not know
or simply forget to update this check as needed.

>                ifp_flags = ifp->flags;
>                if (ifp_flags & IFA_F_OPTIMISTIC) ifp_flags &= ~IFA_F_TENTATIVE;

I think this is cleaner.  I've got a version now that incorporates this.

One more thing I just noticed, though: if the interface and the
address matches the supplied arguments but we don't like the flags we
keep on processing all other addresses on that interface I think.  Is
that right?  (Easy to fix in a follow on patch.)

^ permalink raw reply

* [PATCH] cxgb4: Delete an unnecessary check before the function call "release_firmware"
From: SF Markus Elfring @ 2015-02-04 10:38 UTC (permalink / raw)
  To: Hariprasad S, netdev; +Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <5317A59D.4@users.sourceforge.net>

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 4 Feb 2015 11:28:43 +0100

The release_firmware() function tests whether its argument is NULL and then
returns immediately. Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
index 1147e1e..6d08b8c 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
@@ -5129,8 +5129,7 @@ static int adap_init0(struct adapter *adap)
 				 state, &reset);
 
 		/* Cleaning up */
-		if (fw != NULL)
-			release_firmware(fw);
+		release_firmware(fw);
 		t4_free_mem(card_fw);
 
 		if (ret < 0)
-- 
2.2.2

^ permalink raw reply related

* Re: [PATCH net v3] net: ipv6: allow explicitly choosing optimistic addresses
From: Lorenzo Colitti @ 2015-02-04 10:47 UTC (permalink / raw)
  To: Erik Kline; +Cc: netdev@vger.kernel.org, Hannes Frederic Sowa
In-Reply-To: <CAAedzxoF2gJit1e05UJSOWX2RJwbA1PSbdABK3zgta-XtajJyw@mail.gmail.com>

On Wed, Feb 4, 2015 at 7:38 PM, Erik Kline <ek@google.com> wrote:
> One more thing I just noticed, though: if the interface and the
> address matches the supplied arguments but we don't like the flags we
> keep on processing all other addresses on that interface I think.  Is
> that right?  (Easy to fix in a follow on patch.)

What else would the code do? It loops over all the addresses on the
system in hash table order, not interface-by-interface.

^ 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