Netdev List
 help / color / mirror / Atom feed
* [PATCH] ipv6: address issue in __ip6_append_data
From: Madalin Bucur @ 2015-02-13 15:30 UTC (permalink / raw)
  To: netdev, vyasevich; +Cc: davem, Madalin Bucur

Hello,

I think I've found a problem that allows generic IPv6 traffic to be
sent by the stack with CHECKSUM_PARTIAL to a netdevice that declares
NETIF_F_IPV6_CSUM. The NETIF_F_IPV6_CSUM flag is based on the
NETIF_F_IPV6_CSUM_BIT that is described as referring only to TCP and
UDP but in my test ICMPv6 frames with CHECKSUM_PARTIAL are seen:

NETIF_F_IPV6_CSUM_BIT,          /* Can checksum TCP/UDP over IPV6 */

I've traced the issue to a recent commit that includes this check:

	rt->dst.dev->features & NETIF_F_V6_CSUM

The problem with this is that NETIF_F_V6_CSUM is more than one bit:

#define NETIF_F_V6_CSUM         (NETIF_F_GEN_CSUM | NETIF_F_IPV6_CSUM)

Thus the above check should be either:

	(rt->dst.dev->features & NETIF_F_V6_CSUM) == NETIF_F_V6_CSUM

or probably should use NETIF_F_HW_CSUM only:

	rt->dst.dev->features & NETIF_F_HW_CSUM

The mentioned commit is 32dce968dd987adfb0c00946d78dad9154f64759

> Author: Vlad Yasevich <vyasevich@gmail.com>
> Date:   Sat Jan 31 10:40:18 2015 -0500
>
>	ipv6: Allow for partial checksums on non-ufo packets
>
>	Currntly, if we are not doing UFO on the packet, all UDP
>	packets will start with CHECKSUM_NONE and thus perform full
>	checksum computations in software even if device support
>	IPv6 checksum offloading.

>	Let's start start with CHECKSUM_PARTIAL if the device
>	supports it and we are sending only a single packet at
>	or below mtu size.

>	Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
>	Signed-off-by: David S. Miller <davem@davemloft.net>

Reverting this commit solved the issue, then the changes below were
validated as well. The problem was evidentiated by running ping6
but it should be visible with any IPv6 payload that it is not UDP
nor TCP.

Signed-off-by: Madalin Bucur <madalin.bucur@freescale.com>
---
 net/ipv6/ip6_output.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index d33df4c..7177f63 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -1275,7 +1275,7 @@ emsgsize:
 	 */
 	if (!skb &&
 	    length + fragheaderlen < mtu &&
-	    rt->dst.dev->features & NETIF_F_V6_CSUM &&
+	    (rt->dst.dev->features & NETIF_F_V6_CSUM) == NETIF_F_V6_CSUM &&
 	    !exthdrlen)
 		csummode = CHECKSUM_PARTIAL;
 	/*
-- 
1.7.11.7

^ permalink raw reply related

* Re: [PATCH] bridge: make it possible for packets to traverse the bridge withour hitting netfilter
From: Florian Westphal @ 2015-02-13 16:37 UTC (permalink / raw)
  To: Imre Palik
  Cc: David Miller, bridge, stephen, netdev, linux-kernel, imrep,
	aliguori
In-Reply-To: <54DE2174.6040001@gmail.com>

Imre Palik <imrep.amz@gmail.com> wrote:
> On 02/11/15 23:29, David Miller wrote:
> > If I apply this, someone is going to try to submit a patch for every
> > damn protocol layer to add a stupid hack like this.
> 
> Actually this is one of those patches.  There is already a "stupid hack like this" for iptables and arptables.  (Implemented before git history, and giving me 10% speedup.  Many thanks, whoever did it.)
> 
> I also searched various LKML archives, and it seems the existing "stupid hacks" for iptables and arptables haven't resulted in any related patch submission in the last ten years.  (Or my google-fu is weak.)
> 
> Moreover, I cannot imagine any other reasonable on/off switch for bridge-netfilter than these three.  Of course, my imagination might be lacking there.

Why do you load the bridge netfilter module if you don't want it?
Loading it registers the internal hooks for the call-ip(6)tables and
sabotage hooks with NF_BRIDGE protocol so most of the NF_HOOK(NF_BRIDGE, ...
calls become active.

^ permalink raw reply

* Re: [bisected regression] e1000e: "Detected Hardware Unit Hang"
From: Thomas Jarosch @ 2015-02-13 16:14 UTC (permalink / raw)
  To: Brown, Aaron F
  Cc: Kirsher, Jeffrey T, 'Linux Netdev List', Eric Dumazet,
	e1000-devel
In-Reply-To: <309B89C4C689E141A5FF6A0C5FB2118B783AC840@ORSMSX101.amr.corp.intel.com>

Hi Aaron,

On Thursday, 12. February 2015 23:28:27 Brown, Aaron F wrote:
> I do not have any real info.  I had been asked to try and reproduce some
> unit hangs (maybe for this) recently and did not succeed in producing
> them on the parts I have.  Reading through the thread I see this is
> showing up in a NAT environment.  The port that is getting the unit hang
> in the NAT system?

yes, the e1000e NIC is serving the NATed Windows client.

The setup was outlined here:

    http://marc.info/?l=linux-netdev&m=142133691713824&w=2

> I will make some attempts at replicating this with the port in a NAT and
> or forwarding role.  Has a bug been opened for this?  Or has information
> for this specific unit hang been entered into one of the other unit hang
> bugs opened against e1000e?

I didn't do anything(tm). This report sounds like the same issue:

    http://ehc.ac/p/e1000/bugs/378/

Oliver Wagner wrote the problem started to appear
after updating from kernel 3.5 to 3.8.0.35 (new frag size code).

I just noticed now he wrote he has two identical boxes:

---------------------------------------------------
- Box with symptoms: Router/Firewall, packet forwarding
  between different VLANs on eth0 and eth1
- Box without symptoms: Fileserver, eth0/eth1 bonded
  (VLANs used, but no forwarding)
---------------------------------------------------

So it looks like it's related to forwarding somehow,
I've made the same experience IIRC.

Cheers,
Thomas

^ permalink raw reply

* Re: [PATCH] Revert "smc91x: retrieve IRQ and trigger flags in a modern way"
From: Robert Jarzmik @ 2015-02-13 16:08 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Nicolas Pitre, David S. Miller, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <CACRpkdZGze_2=hWe414Eum_9hpNXgog5vmu6Y4R_zhg0x2z0SQ@mail.gmail.com>

Linus Walleij <linus.walleij@linaro.org> writes:

> On Fri, Feb 13, 2015 at 12:59 AM, Robert Jarzmik <robert.jarzmik@free.fr> wrote:
>
> But isn't the real problem that in the device tree case,
> irq_get_irq_data(ndev->irq) will work becaus parsing an interrupt
> from the device tree populates it correctly in platform_get_irq()
> whereas for the legacy lookup it just fetches the number.
>
> So to me it seems like a weakness in the platform_get_irq()
> helper altogether.
>
> Does the following work? (I can send as a separate patch for
> testing if you like).

Almost. If you replace :
> +    if (r->flags & IORESOURCE_BITS)
with:
> +    if (r && (r->flags & IORESOURCE_BITS))

Then you can push a patch with my:
Tested-by: Robert Jarzmik <robert.jarzmik@free.fr>

Now if you can make it in -rc2 or -rc3, this revert should be forgotten. But if
you can't make it for 3.20, I'll push for the revert.

So I think it's up to you now, and let's see what Gregh says about it.

Cheers.

--
Robert

^ permalink raw reply

* Re: [PATCH] bridge: make it possible for packets to traverse the bridge withour hitting netfilter
From: Imre Palik @ 2015-02-13 16:08 UTC (permalink / raw)
  To: David Miller; +Cc: bridge, stephen, netdev, linux-kernel, imrep, aliguori
In-Reply-To: <20150211.142936.951620487173949333.davem@davemloft.net>

On 02/11/15 23:29, David Miller wrote:
> From: Imre Palik <imrep.amz@gmail.com>
> Date: Tue, 10 Feb 2015 10:32:24 +0100
> 
>> From: "Palik, Imre" <imrep@amazon.de>
>>
>> The netfilter code is made with flexibility instead of performance in mind.
>> So when all we want is to pass packets between different interfaces, the
>> performance penalty of hitting netfilter code can be considerable, even when
>> all the firewalling is disabled for the bridge.
>>
>> This change makes it possible to disable netfilter both on a per bridge basis,
>> or for the whole bridging subsystem.  In the case interesting to us, this can
>> lead to more than 10% speedup compared to the case when only bridge-iptables
>> are disabled.
>>
>> Cc: Anthony Liguori <aliguori@amazon.com>
>> Signed-off-by: Imre Palik <imrep@amazon.de>
> 
> Sorry, no.
> 
> If I apply this, someone is going to try to submit a patch for every
> damn protocol layer to add a stupid hack like this.

Actually this is one of those patches.  There is already a "stupid hack like this" for iptables and arptables.  (Implemented before git history, and giving me 10% speedup.  Many thanks, whoever did it.)

I also searched various LKML archives, and it seems the existing "stupid hacks" for iptables and arptables haven't resulted in any related patch submission in the last ten years.  (Or my google-fu is weak.)

Moreover, I cannot imagine any other reasonable on/off switch for bridge-netfilter than these three.  Of course, my imagination might be lacking there.

> Makw NF_HOOK() faster instead.

As far as I see, flexibility implies trashing the cache/TLB.  So it is impossible to have flexibility and performance at the same time.
(Except possibly with self modifying code, but that has its own set of problems.)

^ permalink raw reply

* Re: [PATCH] Revert "smc91x: retrieve IRQ and trigger flags in a modern way"
From: Robert Jarzmik @ 2015-02-13 16:06 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Nicolas Pitre, David S. Miller, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <CACRpkdZGze_2=hWe414Eum_9hpNXgog5vmu6Y4R_zhg0x2z0SQ@mail.gmail.com>

Linus Walleij <linus.walleij@linaro.org> writes:

> On Fri, Feb 13, 2015 at 12:59 AM, Robert Jarzmik <robert.jarzmik@free.fr> wrote:
>
> But isn't the real problem that in the device tree case,
> irq_get_irq_data(ndev->irq) will work becaus parsing an interrupt
> from the device tree populates it correctly in platform_get_irq()
> whereas for the legacy lookup it just fetches the number.
>
> So to me it seems like a weakness in the platform_get_irq()
> helper altogether.
>
> Does the following work? (I can send as a separate patch for
> testing if you like).

Almost. If you replace :
> +    if (r->flags & IORESOURCE_BITS)
with:
> +    if (r && r->flags & IORESOURCE_BITS)

Then you can push a patch with my:
Tested-by: Robert Jarzmik <robert.jarzmik@free.fr>

Now if you can make it in -rc2 or -rc3, this revert should be forgotten. But if
you can't make it for 3.20, I'll push for the revert.

So I think it's up to you now, and let's see what Gregh says about it.

Cheers.

--
Robert

^ permalink raw reply

* [PATCH] vhost: support upto 509 memory regions
From: Igor Mammedov @ 2015-02-13 15:49 UTC (permalink / raw)
  To: linux-kernel; +Cc: mst, kvm, netdev, pbonzini

since commit
 1d4e7e3 kvm: x86: increase user memory slots to 509

it became possible to use a bigger amount of memory
slots, which is used by memory hotplug for
registering hotplugged memory.
However QEMU aborts if it's used with more than ~60
pc-dimm devices and vhost-net since host kernel
in module vhost-net refuses to accept more than 65
memory regions.

Increasing VHOST_MEMORY_MAX_NREGIONS from 65 to 509
to match KVM_USER_MEM_SLOTS fixes issue for vhost-net.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 drivers/vhost/vhost.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 2ee2826..ecbd7a4 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -29,7 +29,7 @@
 #include "vhost.h"
 
 enum {
-	VHOST_MEMORY_MAX_NREGIONS = 64,
+	VHOST_MEMORY_MAX_NREGIONS = 509,
 	VHOST_MEMORY_F_LOG = 0x1,
 };
 
-- 
1.8.3.1

^ permalink raw reply related

* Re: [PATCH] hso: fix rx parsing logic when skb allocation fails
From: David Miller @ 2015-02-13 15:17 UTC (permalink / raw)
  To: aleksander; +Cc: linux-usb, netdev, j.dumon
In-Reply-To: <1423835462-3454-1-git-send-email-aleksander@aleksander.es>

From: Aleksander Morgado <aleksander@aleksander.es>
Date: Fri, 13 Feb 2015 14:51:02 +0100

> If skb allocation fails once the IP header has been received, the rx state is
> being set to WAIT_SYNC. The logic, though, shouldn't directly return, as the
> buffer may contain a full packet, and therefore the WAIT_SYNC state needs to be
> processed (resetting state to WAIT_IP, clearing rx_buf_size and re-initializing
> rx_buf_missing).
> 
> So, just let the while loop continue so that in the next iteration the WAIT_SYNC
> state cleanly stops the loop. The WAIT_SYNC processing will be done just after
> that, only if the end of packet is flagged.
> 
> Signed-off-by: Aleksander Morgado <aleksander@aleksander.es>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH net] tcp: make sure skb is not shared before using skb_get()
From: David Miller @ 2015-02-13 15:12 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev, ycheng
In-Reply-To: <1423831632.4942.25.camel@edumazet-glaptop2.roam.corp.google.com>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 13 Feb 2015 04:47:12 -0800

> From: Eric Dumazet <edumazet@google.com>
> 
> IPv6 can keep a copy of SYN message using skb_get() in
> tcp_v6_conn_request() so that caller wont free the skb when calling
> kfree_skb() later.
>     
> Therefore TCP fast open has to clone the skb it is queuing in
> child->sk_receive_queue, as all skbs consumed from receive_queue are
> freed using __kfree_skb() (ie assuming skb->users == 1)
>     
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Yuchung Cheng <ycheng@google.com>
> Fixes: 5b7ed0892f2af ("tcp: move fastopen functions to tcp_fastopen.c")

Applied, thanks.

^ permalink raw reply

* [PATCH] hso: fix rx parsing logic when skb allocation fails
From: Aleksander Morgado @ 2015-02-13 13:51 UTC (permalink / raw)
  To: linux-usb-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA
  Cc: j.dumon-x9gZzRpC1QbQT0dZR+AlfA, Aleksander Morgado

If skb allocation fails once the IP header has been received, the rx state is
being set to WAIT_SYNC. The logic, though, shouldn't directly return, as the
buffer may contain a full packet, and therefore the WAIT_SYNC state needs to be
processed (resetting state to WAIT_IP, clearing rx_buf_size and re-initializing
rx_buf_missing).

So, just let the while loop continue so that in the next iteration the WAIT_SYNC
state cleanly stops the loop. The WAIT_SYNC processing will be done just after
that, only if the end of packet is flagged.

Signed-off-by: Aleksander Morgado <aleksander-Dvg4H30XQSRVIjRurl1/8g@public.gmane.org>
---
 drivers/net/usb/hso.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index 6b8efca..9cdfb3f 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -914,7 +914,7 @@ static void packetizeRx(struct hso_net *odev, unsigned char *ip_pkt,
 					/* We got no receive buffer. */
 					D1("could not allocate memory");
 					odev->rx_parse_state = WAIT_SYNC;
-					return;
+					continue;
 				}
 
 				/* Copy what we got so far. make room for iphdr
-- 
2.3.0

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

^ permalink raw reply related

* [PATCH net] tcp: make sure skb is not shared before using skb_get()
From: Eric Dumazet @ 2015-02-13 12:47 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Yuchung Cheng

From: Eric Dumazet <edumazet@google.com>

IPv6 can keep a copy of SYN message using skb_get() in
tcp_v6_conn_request() so that caller wont free the skb when calling
kfree_skb() later.
    
Therefore TCP fast open has to clone the skb it is queuing in
child->sk_receive_queue, as all skbs consumed from receive_queue are
freed using __kfree_skb() (ie assuming skb->users == 1)
    
Signed-off-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Yuchung Cheng <ycheng@google.com>
Fixes: 5b7ed0892f2af ("tcp: move fastopen functions to tcp_fastopen.c")
---
 net/ipv4/tcp_fastopen.c |   32 ++++++++++++++++++++++++--------
 1 file changed, 24 insertions(+), 8 deletions(-)

diff --git a/net/ipv4/tcp_fastopen.c b/net/ipv4/tcp_fastopen.c
index 53db2c309572..ea82fd492c1b 100644
--- a/net/ipv4/tcp_fastopen.c
+++ b/net/ipv4/tcp_fastopen.c
@@ -134,6 +134,7 @@ static bool tcp_fastopen_create_child(struct sock *sk,
 	struct tcp_sock *tp;
 	struct request_sock_queue *queue = &inet_csk(sk)->icsk_accept_queue;
 	struct sock *child;
+	u32 end_seq;
 
 	req->num_retrans = 0;
 	req->num_timeout = 0;
@@ -185,20 +186,35 @@ static bool tcp_fastopen_create_child(struct sock *sk,
 
 	/* Queue the data carried in the SYN packet. We need to first
 	 * bump skb's refcnt because the caller will attempt to free it.
+	 * Note that IPv6 might also have used skb_get() trick
+	 * in tcp_v6_conn_request() to keep this SYN around (treq->pktopts)
+	 * So we need to eventually get a clone of the packet,
+	 * before inserting it in sk_receive_queue.
 	 *
 	 * XXX (TFO) - we honor a zero-payload TFO request for now,
 	 * (any reason not to?) but no need to queue the skb since
 	 * there is no data. How about SYN+FIN?
 	 */
-	if (TCP_SKB_CB(skb)->end_seq != TCP_SKB_CB(skb)->seq + 1) {
-		skb = skb_get(skb);
-		skb_dst_drop(skb);
-		__skb_pull(skb, tcp_hdr(skb)->doff * 4);
-		skb_set_owner_r(skb, child);
-		__skb_queue_tail(&child->sk_receive_queue, skb);
-		tp->syn_data_acked = 1;
+	end_seq = TCP_SKB_CB(skb)->end_seq;
+	if (end_seq != TCP_SKB_CB(skb)->seq + 1) {
+		struct sk_buff *skb2;
+
+		if (unlikely(skb_shared(skb)))
+			skb2 = skb_clone(skb, GFP_ATOMIC);
+		else
+			skb2 = skb_get(skb);
+
+		if (likely(skb2)) {
+			skb_dst_drop(skb2);
+			__skb_pull(skb2, tcp_hdrlen(skb));
+			skb_set_owner_r(skb2, child);
+			__skb_queue_tail(&child->sk_receive_queue, skb2);
+			tp->syn_data_acked = 1;
+		} else {
+			end_seq = TCP_SKB_CB(skb)->seq + 1;
+		}
 	}
-	tcp_rsk(req)->rcv_nxt = tp->rcv_nxt = TCP_SKB_CB(skb)->end_seq;
+	tcp_rsk(req)->rcv_nxt = tp->rcv_nxt = end_seq;
 	sk->sk_data_ready(sk);
 	bh_unlock_sock(child);
 	sock_put(child);

^ permalink raw reply related

* Re: [PATCH RFC v5 net-next 4/6] virtio-net: add basic interrupt coalescing support
From: Pawel Moll @ 2015-02-13 12:41 UTC (permalink / raw)
  To: Rusty Russell
  Cc: Michael S. Tsirkin, Jason Wang, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	virtualization@lists.linux-foundation.org, pagupta@redhat.com,
	Cornelia Huck
In-Reply-To: <874mqqbk12.fsf@rustcorp.com.au>

On Fri, 2015-02-13 at 02:52 +0000, Rusty Russell wrote:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> > On Tue, Feb 10, 2015 at 12:02:37PM +1030, Rusty Russell wrote:
> >> Jason Wang <jasowang@redhat.com> writes:
> >> > This patch enables the interrupt coalescing setting through ethtool.
> >> 
> >> The problem is that there's nothing network specific about interrupt
> >> coalescing.  I can see other devices wanting exactly the same thing,
> >> which means we'd deprecate this in the next virtio standard.
> >> 
> >> I think the right answer is to extend like we did with
> >> vring_used_event(), eg:
> >> 
> >> 1) Add a new feature VIRTIO_F_RING_COALESCE.
> >> 2) Add another a 32-bit field after vring_used_event(), eg:
> >>         #define vring_used_delay(vr) (*(u32 *)((vr)->avail->ring[(vr)->num + 2]))
> >> 
> >> This loses the ability to coalesce by number of frames, but we can still
> >> do number of sg entries, as we do now with used_event, and we could
> >> change virtqueue_enable_cb_delayed() to take a precise number if we
> >> wanted.
> >
> > But do we expect delay to be update dynamically?
> > If not, why not stick it in config space?
> 
> Hmm, we could update it dynamically (and will, in the case of ethtool).
> But it won't be common, so we could append a field to
> virtio_pci_common_cfg for PCI.
> 
> I think MMIO and CCW would be easy to extend too, but CC'd to check.

As far as I understand the virtio_pci_common_cfg principle (just had a
look, for the first time ;-), it's now an equivalent of the MMIO control
registers block. I see no major problem with adding another one.

Or were you thinking about introducing some standard for the "real"
config space? (fine with me as well - the transport will have nothing to
do :-)

Paweł

^ permalink raw reply

* Re: [PATCH net-next 3/3] ipv4: Create probe timer for tcp PMTU as per RFC4821
From: Eric Dumazet @ 2015-02-13 12:31 UTC (permalink / raw)
  To: Fan Du; +Cc: davem, netdev, fengyuleidian0615
In-Reply-To: <1423815405-32644-4-git-send-email-fan.du@intel.com>

On Fri, 2015-02-13 at 16:16 +0800, Fan Du wrote:
> As per RFC4821 7.3.  Selecting Probe Size, a probe timer should
> be armed once probing has converged. Once this timer expired,
> probing again to take advantage of any path PMTU change. The
> recommended probing interval is 10 minutes per RFC1981.
> 
> Signed-off-by: Fan Du <fan.du@intel.com>
> ---
>  include/net/inet_connection_sock.h |    2 ++
>  include/net/netns/ipv4.h           |    1 +
>  include/net/tcp.h                  |    3 +++
>  net/ipv4/sysctl_net_ipv4.c         |    7 +++++++
>  net/ipv4/tcp.c                     |    2 ++
>  net/ipv4/tcp_ipv4.c                |    1 +
>  net/ipv4/tcp_output.c              |   23 ++++++++++++++++++++++-
>  7 files changed, 38 insertions(+), 1 deletions(-)
> 
> diff --git a/include/net/inet_connection_sock.h b/include/net/inet_connection_sock.h
> index 3d0932e..e78e5ab 100644
> --- a/include/net/inet_connection_sock.h
> +++ b/include/net/inet_connection_sock.h
> @@ -126,6 +126,8 @@ struct inet_connection_sock {
>  
>  		int		  search_high_sav;
>  		int		  search_low_sav;
> +
> +		struct timer_list probe_timer;
>  

We certainly wont add yet another timer in tcp socket for such usage.

And a buggy one, since you forgot all the refcounting associated with
such timers.

^ permalink raw reply

* Re: [PATCH] iwl4965: Enable checking of format strings
From: Rasmus Villemoes @ 2015-02-13 12:04 UTC (permalink / raw)
  To: David Laight
  Cc: Mark Rustad, Rustad, Mark D, Stanislaw Gruszka, Kalle Valo,
	linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D1CAE3325@AcuExch.aculab.com>

On Fri, Feb 13 2015, David Laight <David.Laight@ACULAB.COM> wrote:

> From: Rasmus Villemoes
>> Well, probably the linker is allowed to overlap "anonymous" objects
>> (string literals) with whatever const char[] (or indeed any const)
>> object it finds containing the appropriate byte sequence. But I think
>> language lawyers would insist that for
>> 
>> const char foo[] = "a string";
>> const char bar[] = "a string";
>
> A quick test shows those are separate strings.
> But 'const char *foo = "xxx";' will share.

Yes, of course, because in that case you are actually creating two
objects, "xxx" which the linker will find some place to put, and foo
which is initialized to the address of whereever "xxx" was/will be
put. So one is wasting sizeof(const char*). Also, passing foo to a
function means the compiler has to load the value of foo and use that,
instead of simply passing the compile-time (well, link-time) constant
address of "xxx".

> You also need -O1 to get the strings into .rodata.str.n so that the linker
> can merge them.

Sure, optimization has to be turned on, but isn't the kernel always
compiled with -O2? ISTR that there are some things which won't even
work/compile with -O0.

Rasmus

^ permalink raw reply

* [PATCH] com20020-pci: add support for eae single card
From: Michael Grzeschik @ 2015-02-13 12:03 UTC (permalink / raw)
  To: netdev; +Cc: linux-kernel, davem, kernel

Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
 drivers/net/arcnet/com20020-pci.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/net/arcnet/com20020-pci.c b/drivers/net/arcnet/com20020-pci.c
index 945f532..96edc13 100644
--- a/drivers/net/arcnet/com20020-pci.c
+++ b/drivers/net/arcnet/com20020-pci.c
@@ -214,8 +214,17 @@ static struct com20020_pci_card_info card_info_sohard = {
 	.flags = ARC_CAN_10MBIT,
 };
 
-static struct com20020_pci_card_info card_info_eae = {
-	.name = "EAE PLX-PCI",
+static struct com20020_pci_card_info card_info_eae_arc1 = {
+	.name = "EAE PLX-PCI ARC1",
+	.devcount = 1,
+	.chan_map_tbl = {
+		{ 2, 0x00, 0x08 },
+	},
+	.flags = ARC_CAN_10MBIT,
+};
+
+static struct com20020_pci_card_info card_info_eae_ma1 = {
+	.name = "EAE PLX-PCI MA1",
 	.devcount = 2,
 	.chan_map_tbl = {
 		{ 2, 0x00, 0x08 },
@@ -359,9 +368,15 @@ static const struct pci_device_id com20020pci_id_table[] = {
 	},
 	{
 		0x10B5, 0x9050,
+		0x10B5, 0x3263,
+		0, 0,
+		(kernel_ulong_t)&card_info_eae_arc1
+	},
+	{
+		0x10B5, 0x9050,
 		0x10B5, 0x3292,
 		0, 0,
-		(kernel_ulong_t)&card_info_eae
+		(kernel_ulong_t)&card_info_eae_ma1
 	},
 	{
 		0x14BA, 0x6000,
-- 
2.1.4

^ permalink raw reply related

* VLAN creation may lead to deadlock.
From: Ignacy Gawedzki @ 2015-02-13 12:02 UTC (permalink / raw)
  To: netdev

Hi,

I stumbled upon the following annoying situation: supposing eth0 is some
interface that is currently DOWN, the following command

  ip link add link eth0 name eth0.1 up type vlan id 1

never returns and the kernel printks the following lines every few seconds

  unregister_netdevice: waiting for eth0.1 to become free. Usage count = 2

I understand that the kernel should prevent me from creating a VLAN interface
that's initially up when loose-binding is off and the master interface is down
(and indeed there is absolutely no problem when loose-binding is on), but the
unregistering of the currently-being-registered device should not block like
that.  It appears there are too few dev_puts compared to dev_holds in there.

Regards,

Ignacy

-- 
Ignacy Gawędzki
R&D Engineer
Green Communications

^ permalink raw reply

* softlockups when trying to restore an nft set of 1M entries
From: Josh Hunt @ 2015-02-13 11:59 UTC (permalink / raw)
  To: Thomas Graf, Pablo Neira Ayuso, Patrick McHardy; +Cc: netfilter-devel, netdev

In my testing of nftables sets for our netdev bof discussion I came 
across this problem where if I try and do a set restore of 1M entries 
the machine gets into a softlockup state. Once this is triggered the 
system has to be rebooted.

I can trigger the case by generating a simple nft rules file which 
defines a set of type ipv4_addr. Something like this:

flush ruleset
table ip filter {
         set blackhole {
                 type ipv4_addr
         }
         chain input {
                  type filter hook input priority 0;
         }

         chain forward {
                  type filter hook forward priority 0;
         }

         chain output {
                  type filter hook output priority 0;
         }
}

except inside the set definition above I add 1M random ipv4 addresses. 
Running "nft -f <filename>" will reproduce the problem. I also saw this 
when trying to do a restore of 250k entries.

There are a few problems going on from what I can tell. The first is
the set defaults to 4 buckets and during restores the # of buckets does 
not increase. I'm currently investigating to understand why we don't 
expand the set on restores. However my guess into why we're 
softlockuping here is that we're trying to shove 1M entries into 4 
buckets :)

Second, the user has no way to tune the # of initial buckets. My 
patchset "nft hash set expansion fixes" fixes this. If I tune the hash 
to use a reasonable # of buckets for 1M entries. I do not see the 
softlockup problem.

I ran these tests using the current net-next.

Here's some of the softlockup output. Let me know if you'd like more 
info, etc.

[  328.092675] NMI watchdog: BUG: soft lockup - CPU#4 stuck for 22s! 
[nft:3921]
[  328.100185] Modules linked in: nft_hash nft_rbtree nf_tables_ipv4 
nf_tables nfnetlink iptable_filter ip_tables x_tables dm_crypt 
ipmi_devintf ipmi_msghandler i2c_dev ipv6 coretemp hwmon bnx2x ptp 
pps_core i2c_i801 lpc_ich i2c_core mfd_core crc32c_generic crc32c_intel 
ie31200_edac libcrc32c edac_core mdio ext4 jbd2 crc16 raid10 raid456 
async_raid6_recov async_pq rai�6_pq async_xor xor async_memcpy async_tx 
raid1 raid0 linear md_mod dm_mod ahci libahci libata mpt2sas 
scsi_transport_sas raid_class
[  328.151902] CPU: 4 PID: 3921 Comm: nft Not tainted 3.19.0-rc7+ #28
[  328.158542] Hardware name: CIARA TECHNOLOGIES 1X8-X6 SSD 16G 
10GE/S5530WG2NR-LE-2T-AKA, BIOS 7.008 14/04/2014
[  328.169289] task: ffff880407266210 ti: ffff880400ff0000 task.ti: 
ffff880400ff0000
[  328.177609] RIP: 0010:[<ffffffff8134dd41>]  [<ffffffff8134dd41>] 
memcmp+0x11/0x50
[  328.186043] RSP: 0018:ffff880400ff38d8  EFLAGS: 00000202
[  328.191811] RAX: 00000000000000f4 RBX: ffff88040f000340 RCX: 
00000000000000e3
[  328.199407] RDX: 0000000000000004 RSI: ffff880400ff39f0 RDI: 
ffff8803f37ce7e8
[  328.207000] RBP: ffff880400ff38d8 R08: 00000000000000d9 R09: 
00000000ffffffdf
[  328.214593] R10: 0000000000000015 R11: dead000000100100 R12: 
000412d000000010
[  328.222189] R13: 00000040�000000b R14: ffffffff000492d0 R15: 
ffff880400ff3928
[  328.229781] FS:  00007f7ddf1d6700(0000) GS:ffff88041fd00000(0000) 
knlGS:0000000000000000
[  328.238709] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  328.244909] CR2: 00007f3b0d890000 CR3: 000000040ae41000 CR4: 
00000000001407e0
[  328.252505] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 
0000000000000000
[  328.260100] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 
0000000000000400
[  328.267692] Stack:
[  328.270171]  ffff880400ff3908 ffffffffa056160a ffff880400ff38f8 
ffff8800379b2290
[  328.278805]  ffffffffa05615d0 ffff880400ff3968 ffff880400ff3958 
ffffffff8135a25d
[  328.287437]  ffff88040c86a300 0495cff0a054a125 0000000000000000 
ffff8800379b2200
[  328.296070] Call Trace:
[  328.298983]  [<ffffffffa056160a>] nft_hash_compare+0x3a/0x88 [nft_hash]
[  328.306054]  [<ffffffffa05615d0>] ? nft_hash_lookup+0x60/0x60 [nft_hash]
[  328.313218]  [<ffffffff8135a25d>] rhashtable_lookup_compare+0x6d/0xb0
[  328.320118]  [<ffffffffa0561560>] nft_has�_get+0x30/0x40 [nft_hash]
[  328.326846]  [<ffffffffa054a4d4>] nft_add_set_elem+0x164/0x3b0 
[nf_tables]
[  328.334180]  [<ffffffffa0546fdc>] ? nft_trans_set_add+0x2c/0xa0 
[nf_tables]
[  328.341602]  [<ffffffffa0561000>] ? 0xffffffffa0561000
[  328.347205]  [<ffffffffa054d85f>] ? nf_tables_newset+0x7df/0x8d0 
[nf_tables]
[  328.354711]  [<ffffffff8136ca52>] ? nla_strcmp+0x42/0x50
[  328.360489]  [<ffffffffa0546b14>] ? nf_tables_table_lookup+0x44/0x80 
[nf_tables]
[  328.368723]  [<ffffffffa054da1e>] nf_tables_newsetelem+0xce/0x170 
[nf_tables]
[  328.376316]  [<ffffffffa054093c>] nfnetlink_rcv_batch+0x33c/0x430 
[nfnetlink]
[  328.383913]  [<ffffffffa05406ed>] ? nfnetlink_rcv_batch+0xed/0x430 
[nfnetlink]
[  328.391974]  [<ffffffffa0540abf>] nfnetlink_rcv+0x8f/0xc8 [nfnetlink]
[  328.398876]  [<ffffffff81568a92>] netlink_unicast+0x182/0x210
[  328.405082]  [<ffffffff81568f58>] netlink_sendmsg+0x378/0x3e0
[  328.411295]  [<ffffffff8151ec2f>] do_sock_sendmsg+0x8f/0xa0
[  328.417327]  [<ffffffff8151ec50>] sock_sendmsg+0x10/0x20
[  328.423097]  [<ffffffff81521655>] ___sys_sendmsg+0x315/0x330
[  328.429216]  [<ffffffff810daacc>] ? acct_account_cputime+0x1c/0x20
[  328.435859]  [<ffffffff81078f5d>] ? account_system_time+0x9d/0x190
[  328.442502]  [<ffffffff81078a55>] ? local_clock+0x25/0x30
[  328.448364]  [<ffffffff8109faf8>] ? rcu_eqs_enter+0x68/0x90
[  328.454399]  [<ffffffff810daacc>] ? acct_account_cputime+0x1c/0x20
[  328.461042]  [<ffffffff81078eb1>] ? account_user_time+0x91/0xa0
[  328.467423]  [<ffffffff81522469>] __sys_sendmsg+0x49/0x90
[  328.473287]  [<ffffffff81616dfd>] ? int_check_syscall_exit_work+0x34/0x3d
[  328.480534]  [<ffffffff815224c9>] SyS_sendmsg+0x19/0x20
[  328.486223]  [<ffffffff81616bd2>] system_call_fastpath+0x12/0x17
[  328.492690] Code: c3 66 0f 1f 84 00 00 00 00 00 31 c0 c6 06 00 5d c3 
66 0f 1f 84 00 00 00 00 00 55 31 c0 48 85 d2 48 89 e5 74 2f 0f b6 07 0f 
b6 0e <29> c8 75 25 48 83 ea 01 31 c9 eb 18 0f 1f 00 44 0f b6 4c 0f 01
[  331.718616] INFO: rcu_sched self-detected stall on CPU[  331.720614] 
INFO: rcu_sched detected stalls on CPUs/tasks: { 4} (detected by 0, 
t=30002 jiffies, g=6997, c=6996, q=0)
[  331.720617] Task dump for CPU 4:
[  331.720618] nft             R  running task        0  3921   3876 
0x00080008
[  331.720620]  ffff88041fffad80 000000000001a5e8 000000000000003e 
000000000000003f
[  331.720621]  0000000000000000 ffff8803f41ac000 ffff88040f000340 
0000000000000000
[  331.720622]  0000000000000000 ffff88040f0012c0 ffff88040f000340 
ffff880400ff3818
[  331.720623] Call Trace:
[  331.720625]  [<ffffffff8116d593>] ? kmem_getpages+0xb3/0x110
[  331.720629]  [<ffffffff8116ec26>] ? cache_grow+0x146/0x210
[  331.720630]  [<ffffffff8134dd3e>] ? memcmp+0xe/0x50
[  331.720634]  [<ffffffff8136ccf0>] ? nla_parse+0x90/0x110
[  331.720636]  [<ffffffffa056160a>] ? nft_hash_compare+0x3a/0x88 [nft_hash]
[  331.720638]  [<ffffffffa05615d0>] ? nft_hash_lookup+0x60/0x60 [nft_hash]
[  331.720639]  [<ffffffff8135a25d>] ? rhashtable_lookup_compare+0x6d/0xb0
[  331.720641]  [<ffffffffa0�61560>] ? nf�_hash_get+0x30/0x40 [nft_hash]
[  331.720642]  [<ffffffffa054a4d4>] ? nft_add_set_elem+0x164/0x3b0 
[nf_tables]
[  331.720645]  [<ffffffffa0546fdc>] ? nft_trans_set_add+0x2c/0xa0 
[nf_tables]
[  331.720647]  [<ffffffffa0561000>] ? 0xffffffffa0561000
[  331.720654]  [<ffffffffa054d85f>] ? nf_tables_newset+0x7df/0x8d0 
[nf_tables]
[  331.720656]  [<ffffffff8136ca52>] ? nla_strcmp+0x42/0x50
[  331.720657]  [<ffffffffa0546b14>] ? nf_tables_table_lookup+0x44/0x80 
[nf_tables]
[  331.720659]  [<ffffffffa054da1e>] ? nf_tables_newsetelem+0xce/0x170 
[nf_tables]
[  331.720661]  [<ffffffffa054093c>] ? nfnetlink_rcv_atch+0x33c/0x430 
[nfnetlink]
[  331.720663]  [<ffffffffa05406ed>] ? nfnetlink_rcv_batch+0xed/0x430 
[nfnetlink]
[  331.720664]  [<ffffffffa0540abf>] ? nfnetlink_rcv+0x8f/0xc8 [nfnetlink]
[  331.720665]  [<ffffffff81568a92>] ? netlink_unicast+0x182/0x210
[  331.720668]  [<ffffffff81568f58>] ? netlink_sendmsg+0x378/0x3e0
[  331.720670]  [<ffffffff8151ec2f>] ? do_sock_sendmsg+0x8f/0xa0
[  331.720672]  [<ffffffff8151ec50>] ? sock_sendmsg+0x10/0x20
[  331.720673]  [<ffffffff81521655>] ? ___sys_sendmsg+0x315/0x330
[  331.720675]  [<ffffffff810daacc>] ? acct_account_cputime+0x1c/0x20
[  331.720677]  [<ffffffff81078f5d>] ? account_system_time+0x9d/0x190
[  331.720679]  [<ffffffff81078a55>] ? local_clock+0x25/0x30
[  331.720680]  [<ffffffff8109faf8>] ? rcu_eqs_enter+0x68/0x90
[  331.720683]  [<ffffffff810daacc>] ? acct_account_cputime+0x1c/0x20
[  331.720684]  [<ffffffff81078eb1>] ? account_user_time+0x91/0xa0
[  331.720685]  [<ffffffff81522469>] ? __sys_sendmsg+0x49/0x90
[  331.720687]  [<ffffffff81616dfd>] ? int_check_syscall_exit_work+0x34/0x3d
[  331.720690]  [<ffffffff815224c9>] ? SyS_sendmsg+0x19/0x20
[  331.720691]  [<ffffffff81616bd2>] ? system_call_fastpath+0x12/0x17

Thanks
Josh
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* vnet problem (bug? feature?)
From: Toerless Eckert @ 2015-02-13 11:41 UTC (permalink / raw)
  To: netdev

Hope this is the right mailing list, if not, then which one ? (thanks)

- Created vnet pair
- NOT putting them into different namespaces.
- Unicast across them works fine.
- When sending IP multicsast into one end, i can not receive it on the other side
  (with normal socket API applications).

Interestingly:
- When using tcpdump (pcap), i can actually see the multicast packets
  both when binding to the sending vnet interface and when binding to
  the receiving vnet.
- When using tcpdump and sending unicast, tcpdump doesn't show me any packets ;-(

When putting one vnet interface into a separate namespace, i can send
multicast packets. I wonder if what i see is not a result of namespaces
but just how multicast packets interact with the routing table(s) (single
one when both vnet interfaces are in the same namespace).

Tihis is unfortunately all with an old kernel (3.7), i'll try to get a newer
kernel on some PC, a bit constrained on available HW, so thought i dare to
ask before having tested on the latest kernel.

(btw: Is there any api to put an interface back from a namespace into default ?)

Thanks!
    Toerless

^ permalink raw reply

* RE: [PATCH] iwl4965: Enable checking of format strings
From: David Laight @ 2015-02-13 11:20 UTC (permalink / raw)
  To: 'Rasmus Villemoes', Mark Rustad
  Cc: Rustad, Mark D, Stanislaw Gruszka, Kalle Valo,
	linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <87y4o2xfgz.fsf@rasmusvillemoes.dk>

From: Rasmus Villemoes
> Well, probably the linker is allowed to overlap "anonymous" objects
> (string literals) with whatever const char[] (or indeed any const)
> object it finds containing the appropriate byte sequence. But I think
> language lawyers would insist that for
> 
> const char foo[] = "a string";
> const char bar[] = "a string";

A quick test shows those are separate strings.
But 'const char *foo = "xxx";' will share.
You also need -O1 to get the strings into .rodata.str.n so that the linker
can merge them.

	David

^ permalink raw reply

* [PATCH iproute2] ss: Fix filter expression parser
From: Vadim Kochan @ 2015-02-13 11:01 UTC (permalink / raw)
  To: netdev; +Cc: Vadim Kochan

From: Vadim Kochan <vadim4j@gmail.com>

Seems expression parser did not work correctly some
long time and such simple things did not work too:

    # ss -a '( sport = :ssh )'

Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
---
 misc/ss.c | 51 ++++++++++++++++++++++++++++++++-------------------
 1 file changed, 32 insertions(+), 19 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index 0a6a65e..eb4e0ec 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -240,10 +240,11 @@ static void filter_db_set(struct filter *f, int db)
 
 static void filter_af_set(struct filter *f, int af)
 {
-	f->dbs	    |= default_afs[af].dbs;
-	f->states   |= default_afs[af].states;
-	f->families |= 1 << af;
-	do_default   = 0;
+	f->dbs		   |= default_afs[af].dbs;
+	f->states	   |= default_afs[af].states;
+	f->families	   |= 1 << af;
+	do_default	    = 0;
+	preferred_family    = af;
 }
 
 static int filter_af_get(struct filter *f, int af)
@@ -1316,15 +1317,12 @@ static int xll_name_to_index(const char *dev)
 void *parse_hostcond(char *addr)
 {
 	char *port = NULL;
-	struct aafilter a;
+	struct aafilter a = { .port = -1 };
 	struct aafilter *res;
-	int fam = 0;
+	int fam = preferred_family;
 	struct filter *f = &current_filter;
 
-	memset(&a, 0, sizeof(a));
-	a.port = -1;
-
-	if (filter_af_get(f, AF_UNIX) || strncmp(addr, "unix:", 5) == 0) {
+	if (fam == AF_UNIX || strncmp(addr, "unix:", 5) == 0) {
 		char *p;
 		a.addr.family = AF_UNIX;
 		if (strncmp(addr, "unix:", 5) == 0)
@@ -1336,7 +1334,7 @@ void *parse_hostcond(char *addr)
 		goto out;
 	}
 
-	if (filter_af_get(f, AF_PACKET) || strncmp(addr, "link:", 5) == 0) {
+	if (fam == AF_PACKET || strncmp(addr, "link:", 5) == 0) {
 		a.addr.family = AF_PACKET;
 		a.addr.bitlen = 0;
 		if (strncmp(addr, "link:", 5) == 0)
@@ -1362,7 +1360,7 @@ void *parse_hostcond(char *addr)
 		goto out;
 	}
 
-	if (filter_af_get(f, AF_NETLINK) || strncmp(addr, "netlink:", 8) == 0) {
+	if (fam == AF_NETLINK || strncmp(addr, "netlink:", 8) == 0) {
 		a.addr.family = AF_NETLINK;
 		a.addr.bitlen = 0;
 		if (strncmp(addr, "netlink:", 8) == 0)
@@ -1388,12 +1386,14 @@ void *parse_hostcond(char *addr)
 		goto out;
 	}
 
-	if (filter_af_get(f, AF_INET) || !strncmp(addr, "inet:", 5)) {
-		addr += 5;
+	if (fam == AF_INET || !strncmp(addr, "inet:", 5)) {
 		fam = AF_INET;
-	} else if (filter_af_get(f, AF_INET6) || !strncmp(addr, "inet6:", 6)) {
-		addr += 6;
+		if (!strncmp(addr, "inet:", 5))
+			addr += 5;
+	} else if (fam == AF_INET6 || !strncmp(addr, "inet6:", 6)) {
 		fam = AF_INET6;
+		if (!strncmp(addr, "inet6:", 6))
+			addr += 6;
 	}
 
 	/* URL-like literal [] */
@@ -1461,8 +1461,11 @@ void *parse_hostcond(char *addr)
 	}
 
 out:
-	if (fam)
+	if (fam != AF_UNSPEC) {
+		f->families = 0;
 		filter_af_set(f, fam);
+		filter_merge(f, f, 0);
+	}
 
 	res = malloc(sizeof(*res));
 	if (res)
@@ -2212,6 +2215,9 @@ static int tcp_show(struct filter *f, int socktype)
 	char *buf = NULL;
 	int bufsize = 64*1024;
 
+	if (!filter_af_get(f, AF_INET) && !filter_af_get(f, AF_INET6))
+		return 0;
+
 	dg_proto = TCP_PROTO;
 
 	if (getenv("TCPDIAG_FILE"))
@@ -2331,6 +2337,9 @@ static int udp_show(struct filter *f)
 {
 	FILE *fp = NULL;
 
+	if (!filter_af_get(f, AF_INET) && !filter_af_get(f, AF_INET6))
+		return 0;
+
 	dg_proto = UDP_PROTO;
 
 	if (!getenv("PROC_NET_UDP") && !getenv("PROC_ROOT")
@@ -2367,6 +2376,9 @@ static int raw_show(struct filter *f)
 {
 	FILE *fp = NULL;
 
+	if (!filter_af_get(f, AF_INET) && !filter_af_get(f, AF_INET6))
+		return 0;
+
 	dg_proto = RAW_PROTO;
 
 	if (f->families&(1<<AF_INET)) {
@@ -3579,8 +3591,6 @@ int main(int argc, char *argv[])
 			state_filter &= ~scan_state(*argv);
 			saw_states = 1;
 		} else {
-			if (ssfilter_parse(&current_filter.f, argc, argv, filter_fp))
-				usage();
 			break;
 		}
 		argc--; argv++;
@@ -3630,6 +3640,9 @@ int main(int argc, char *argv[])
 		exit(0);
 	}
 
+	if (ssfilter_parse(&current_filter.f, argc, argv, filter_fp))
+		usage();
+
 	netid_width = 0;
 	if (current_filter.dbs&(current_filter.dbs-1))
 		netid_width = 5;
-- 
2.2.2

^ permalink raw reply related

* Re: [PATCH 1/1] tun: change speed from 10M to dynamically configured
From: Sergei Shtylyov @ 2015-02-13 10:45 UTC (permalink / raw)
  To: Zhu Yanjun, netdev, mst, jasowang, viro, davem, jonathon.reinhart
In-Reply-To: <1423808353-8722-2-git-send-email-Yanjun.Zhu@windriver.com>

On 2/13/2015 9:19 AM, Zhu Yanjun wrote:

> The default speed of normal nic is 1000M while the default speed
> of tun is 10M. Now the default speed of tun is changed to 1000M.
> And there are 3 options: 10M, 100M and 1000M to the speed of tun.
> The command "ethtool -s tun0 speed 10/100/1000" can configure the
> speed of tun dynamically.

> CC: Michael S. Tsirkin <mst@redhat.com>
> CC: Jason Wang <jasowang@redhat.com>
> CC: Al Viro <viro@zeniv.linux.org.uk>
> Reviewed-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> Reviewed-by: Jonathon Reinhart <jonathon.reinhart@gmail.com>
> Signed-off-by: Zhu Yanjun <Yanjun.Zhu@windriver.com>

[...]

> diff --git a/include/uapi/linux/if_tun.h b/include/uapi/linux/if_tun.h
> index 50ae243..78a09a7 100644
> --- a/include/uapi/linux/if_tun.h
> +++ b/include/uapi/linux/if_tun.h
> @@ -66,6 +66,11 @@
>   #define IFF_PERSIST	0x0800
>   #define IFF_NOFILTER	0x1000
>
> +/*add speed control, default 1000M*/

    Please do add spaces after /* and before */.

> +#define TUN_CTRL_SPD_10         0x0020
> +#define TUN_CTRL_SPD_100        0x0040
> +#define TUN_CTRL_SPD_1000       0x0080
> +
>   /* Socket options */
>   #define TUN_TX_TIMESTAMP 1

WBR, Sergei

^ permalink raw reply

* Re: [PATCH] iwl4965: Enable checking of format strings
From: Rasmus Villemoes @ 2015-02-13 10:39 UTC (permalink / raw)
  To: Mark Rustad
  Cc: Rustad, Mark D, Stanislaw Gruszka, Kalle Valo,
	linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <54DDAE03.4000502@gmail.com>

On Fri, Feb 13 2015, Mark Rustad <mrustad@gmail.com> wrote:

> On 2/12/15 2:20 AM, Rasmus Villemoes wrote:
>> Rather weak arguments, but I have three of them :-)
>
> Yes, weak. All three.
>
>> (1) If I'm reading some code and spot a non-constant format
>> argument, I sometimes track back to see how e.g. fmt_value is
>> defined. If I then see it's a macro, I immediately think "ok, the
>> compiler is doing type-checking". If it is a const char[], I have
>> to remember that gcc also does it in that case (as opposed to for
>> example const char*const).
>
> GCC should check in both cases. The case you are replacing was not
> const char * const, but only const char *. Still, the compiler really
> should check either form, even though theoretically the pointer in the
> latter case could be changed, but the initial const value should be a
> good indication of what the parameters are expected to be. No real
> reason for the compiler not to check it.

I agree with all of that - just wanted to point out what gcc currently
does and doesn't, and changing to const char[] would indeed enable
checking.

>> (2) The names of these variables themselves may end up wasting a
>> few bytes in the image.
>
> Maybe in a debug image, but they should be stripped from any normal
> image. Really not a factor.

Sure, that was by far the weakest, and let's ignore that.

>> (3) gcc/the linker doesn't merge identical const char[] arrays
>> across translation units. It also doesn't consider their tails for
>> merging with string literals. So although these specific strings
>> are unlikely to appear elsewhere, a string such as "%10u\n" or
>> "max\n" couldn't be merged with one of the above.
>
> I haven't checked, but there is no theoretical reason that const char
> [] items could not be merged exactly as the literals are. Considering
> the boundaries the compiler guys push on optimization, doing such
> merging would be tame by comparison (speculative stores make me crazy).

Well, probably the linker is allowed to overlap "anonymous" objects
(string literals) with whatever const char[] (or indeed any const)
object it finds containing the appropriate byte sequence. But I think
language lawyers would insist that for

const char foo[] = "a string";
const char bar[] = "a string";

foo and bar have different addresses, whether they are defined in the
same or different TUs. One could then argue that if their address is
never taken explicitly it should be ok, but since passing foo to a printf
function effectively makes the address of foo escape the TU (even though
one is formally passing pointer to first element), I can certainly see
why compiler people would be reluctant to do merging of such objects.

Rasmus

^ permalink raw reply

* Re: [PATCH V2 1/1] tun: change speed from 10M to dynamically configured
From: Sergei Shtylyov @ 2015-02-13 10:37 UTC (permalink / raw)
  To: Zhu Yanjun, netdev, mst, jasowang, viro, davem
In-Reply-To: <1423798552-7091-2-git-send-email-Yanjun.Zhu@windriver.com>

Hello.

On 2/13/2015 6:35 AM, Zhu Yanjun wrote:

> The default speed of normal nic is 1000M while the default speed
> of tun is 10M. Now the default speed of tun is changed to 1000M.
> And there are 3 options: 10M, 100M and 1000M to the speed of tun.
> The command "ethtool -s tun0 speed 10/100/1000" can configure the
> speed of tun dynamically.

> CC: Michael S. Tsirkin <mst@redhat.com>
> CC: Jason Wang <jasowang@redhat.com>
> CC: Al Viro <viro@zeniv.linux.org.uk>
> Reviewed-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> Signed-off-by: Zhu Yanjun <Yanjun.Zhu@windriver.com>
> ---
>   drivers/net/tun.c           | 42 +++++++++++++++++++++++++++++++++++++++++-
>   include/uapi/linux/if_tun.h |  5 +++++
>   2 files changed, 46 insertions(+), 1 deletion(-)

> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index 8c8dc16..0ee36f1 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
[...]
> @@ -2257,9 +2260,18 @@ static struct miscdevice tun_miscdev = {
>
>   static int tun_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
>   {
> +	struct tun_struct *tun = netdev_priv(dev);
> +
> +	/* Get the speed of tun */
> +	if (tun->flags & TUN_CTRL_SPD_1000) {
> +		ethtool_cmd_speed_set(cmd, SPEED_1000);
> +	} else if (tun->flags & TUN_CTRL_SPD_100) {
> +		ethtool_cmd_speed_set(cmd, SPEED_100);
> +	} else
> +		ethtool_cmd_speed_set(cmd, SPEED_10);

    I probably missed it in the previous version: {} not needed here at all. 
Please remove to comply with Documentation/CodingStyle.

[...]

WBR, Sergei

^ permalink raw reply

* Re: [PATCH 1/1] tun: change speed from 10M to dynamically configured
From: yzhu1 @ 2015-02-13 10:03 UTC (permalink / raw)
  To: netdev, mst, jasowang, viro, davem, sergei.shtylyov,
	jonathon.reinhart, maxk
In-Reply-To: <1423808353-8722-2-git-send-email-Yanjun.Zhu@windriver.com>

add maxk@qti.qualcomm.com, the maintainer of tun/tap driver.

Zhu Yanjun
On 02/13/2015 02:19 PM, Zhu Yanjun wrote:
> The default speed of normal nic is 1000M while the default speed
> of tun is 10M. Now the default speed of tun is changed to 1000M.
> And there are 3 options: 10M, 100M and 1000M to the speed of tun.
> The command "ethtool -s tun0 speed 10/100/1000" can configure the
> speed of tun dynamically.
>
> CC: Michael S. Tsirkin <mst@redhat.com>
> CC: Jason Wang <jasowang@redhat.com>
> CC: Al Viro <viro@zeniv.linux.org.uk>
> Reviewed-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> Reviewed-by: Jonathon Reinhart <jonathon.reinhart@gmail.com>
> Signed-off-by: Zhu Yanjun <Yanjun.Zhu@windriver.com>
> ---
>   drivers/net/tun.c           | 43 ++++++++++++++++++++++++++++++++++++++++++-
>   include/uapi/linux/if_tun.h |  5 +++++
>   2 files changed, 47 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index 8c8dc16..423b276 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -950,6 +950,9 @@ static void tun_net_init(struct net_device *dev)
>   		dev->addr_len = 0;
>   		dev->mtu = 1500;
>   
> +		/* Set default speed 1000M */
> +		tun->flags |= TUN_CTRL_SPD_1000;
> +
>   		/* Zero header length */
>   		dev->type = ARPHRD_NONE;
>   		dev->flags = IFF_POINTOPOINT | IFF_NOARP | IFF_MULTICAST;
> @@ -2257,9 +2260,18 @@ static struct miscdevice tun_miscdev = {
>   
>   static int tun_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
>   {
> +	struct tun_struct *tun = netdev_priv(dev);
> +
> +	/* Get the speed of tun */
> +	if (tun->flags & TUN_CTRL_SPD_1000) {
> +		ethtool_cmd_speed_set(cmd, SPEED_1000);
> +	} else if (tun->flags & TUN_CTRL_SPD_100) {
> +		ethtool_cmd_speed_set(cmd, SPEED_100);
> +	} else
> +		ethtool_cmd_speed_set(cmd, SPEED_10);
> +
>   	cmd->supported		= 0;
>   	cmd->advertising	= 0;
> -	ethtool_cmd_speed_set(cmd, SPEED_10);
>   	cmd->duplex		= DUPLEX_FULL;
>   	cmd->port		= PORT_TP;
>   	cmd->phy_address	= 0;
> @@ -2287,6 +2299,34 @@ static void tun_get_drvinfo(struct net_device *dev, struct ethtool_drvinfo *info
>   	}
>   }
>   
> +static int tun_set_settings(struct net_device *dev, struct ethtool_cmd *cmd)
> +{
> +	struct tun_struct *tun = netdev_priv(dev);
> +	u32 speed = ethtool_cmd_speed(cmd);
> +	int ret = 0;
> +
> +	/* Set speed flag */
> +	switch (speed) {
> +	case 10:
> +		tun->flags &= ~(TUN_CTRL_SPD_100 | TUN_CTRL_SPD_1000);
> +		tun->flags |= TUN_CTRL_SPD_10;
> +		break;
> +	case 100:
> +		tun->flags &= ~(TUN_CTRL_SPD_10 | TUN_CTRL_SPD_1000);
> +		tun->flags |= TUN_CTRL_SPD_100;
> +		break;
> +	case 1000:
> +		tun->flags &= ~(TUN_CTRL_SPD_10 | TUN_CTRL_SPD_100);
> +		tun->flags |= TUN_CTRL_SPD_1000;
> +		break;
> +	default:
> +		ret = -EINVAL;
> +		tun_debug(KERN_INFO, tun, "wrong speed!\n");
> +	}
> +
> +	return ret;
> +}
> +
>   static u32 tun_get_msglevel(struct net_device *dev)
>   {
>   #ifdef TUN_DEBUG
> @@ -2307,6 +2347,7 @@ static void tun_set_msglevel(struct net_device *dev, u32 value)
>   
>   static const struct ethtool_ops tun_ethtool_ops = {
>   	.get_settings	= tun_get_settings,
> +	.set_settings	= tun_set_settings,
>   	.get_drvinfo	= tun_get_drvinfo,
>   	.get_msglevel	= tun_get_msglevel,
>   	.set_msglevel	= tun_set_msglevel,
> diff --git a/include/uapi/linux/if_tun.h b/include/uapi/linux/if_tun.h
> index 50ae243..78a09a7 100644
> --- a/include/uapi/linux/if_tun.h
> +++ b/include/uapi/linux/if_tun.h
> @@ -66,6 +66,11 @@
>   #define IFF_PERSIST	0x0800
>   #define IFF_NOFILTER	0x1000
>   
> +/*add speed control, default 1000M*/
> +#define TUN_CTRL_SPD_10         0x0020
> +#define TUN_CTRL_SPD_100        0x0040
> +#define TUN_CTRL_SPD_1000       0x0080
> +
>   /* Socket options */
>   #define TUN_TX_TIMESTAMP 1
>   

^ permalink raw reply

* Re: [PATCH net-next 3/3] ipv4: Create probe timer for tcp PMTU as per RFC4821
From: Ying Xue @ 2015-02-13  9:59 UTC (permalink / raw)
  To: Fan Du, davem; +Cc: netdev, fengyuleidian0615
In-Reply-To: <1423815405-32644-4-git-send-email-fan.du@intel.com>

On 02/13/2015 04:16 PM, Fan Du wrote:
> As per RFC4821 7.3.  Selecting Probe Size, a probe timer should
> be armed once probing has converged. Once this timer expired,
> probing again to take advantage of any path PMTU change. The
> recommended probing interval is 10 minutes per RFC1981.
> 
> Signed-off-by: Fan Du <fan.du@intel.com>
> ---
>  include/net/inet_connection_sock.h |    2 ++
>  include/net/netns/ipv4.h           |    1 +
>  include/net/tcp.h                  |    3 +++
>  net/ipv4/sysctl_net_ipv4.c         |    7 +++++++
>  net/ipv4/tcp.c                     |    2 ++
>  net/ipv4/tcp_ipv4.c                |    1 +
>  net/ipv4/tcp_output.c              |   23 ++++++++++++++++++++++-
>  7 files changed, 38 insertions(+), 1 deletions(-)
> 
> diff --git a/include/net/inet_connection_sock.h b/include/net/inet_connection_sock.h
> index 3d0932e..e78e5ab 100644
> --- a/include/net/inet_connection_sock.h
> +++ b/include/net/inet_connection_sock.h
> @@ -126,6 +126,8 @@ struct inet_connection_sock {
>  
>  		int		  search_high_sav;
>  		int		  search_low_sav;
> +
> +		struct timer_list probe_timer;
>  
>  		/* Information on the current probe. */
>  		int		  probe_size;
> diff --git a/include/net/netns/ipv4.h b/include/net/netns/ipv4.h
> index dbe2254..bb2c2d1 100644
> --- a/include/net/netns/ipv4.h
> +++ b/include/net/netns/ipv4.h
> @@ -84,6 +84,7 @@ struct netns_ipv4 {
>  	int sysctl_tcp_fwmark_accept;
>  	int sysctl_tcp_mtu_probing;
>  	int sysctl_tcp_base_mss;
> +	u32 sysctl_tcp_probe_interval;
>  
>  	struct ping_group_range ping_group_range;
>  
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index 7b57e5b..16fa2e6 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -67,6 +67,9 @@ void tcp_time_wait(struct sock *sk, int state, int timeo);
>  /* The least MTU to use for probing */
>  #define TCP_BASE_MSS		1024
>  
> +/* probing interval, default to 10 minutes as per RFC4821 */
> +#define TCP_PROBE_INTERVAL	600
> +
>  /* After receiving this amount of duplicate ACKs fast retransmit starts. */
>  #define TCP_FASTRETRANS_THRESH 3
>  
> diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
> index d151539..4fa5d31 100644
> --- a/net/ipv4/sysctl_net_ipv4.c
> +++ b/net/ipv4/sysctl_net_ipv4.c
> @@ -883,6 +883,13 @@ static struct ctl_table ipv4_net_table[] = {
>  		.mode		= 0644,
>  		.proc_handler	= proc_dointvec,
>  	},
> +	{
> +		.procname	= "tcp_probe_interval",
> +		.data		= &init_net.ipv4.sysctl_tcp_probe_interval,
> +		.maxlen		= sizeof(int),
> +		.mode		= 0644,
> +		.proc_handler	= proc_dointvec,
> +	},
>  	{ }
>  };
>  
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 9d72a0f..46413ee 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -1986,6 +1986,7 @@ void tcp_close(struct sock *sk, long timeout)
>  	struct sk_buff *skb;
>  	int data_was_unread = 0;
>  	int state;
> +	struct inet_connection_sock *icsk = inet_csk(sk);
>  
>  	lock_sock(sk);
>  	sk->sk_shutdown = SHUTDOWN_MASK;
> @@ -2149,6 +2150,7 @@ adjudge_to_death:
>  	/* Otherwise, socket is reprieved until protocol close. */
>  
>  out:
> +	del_timer(&icsk->icsk_mtup.probe_timer);
>  	bh_unlock_sock(sk);
>  	local_bh_enable();
>  	sock_put(sk);
> diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
> index 5a2dfed..3cc71b3 100644
> --- a/net/ipv4/tcp_ipv4.c
> +++ b/net/ipv4/tcp_ipv4.c
> @@ -2460,6 +2460,7 @@ static int __net_init tcp_sk_init(struct net *net)
>  	}
>  	net->ipv4.sysctl_tcp_ecn = 2;
>  	net->ipv4.sysctl_tcp_base_mss = TCP_BASE_MSS;
> +	net->ipv4.sysctl_tcp_probe_interval = TCP_PROBE_INTERVAL;
>  	return 0;
>  
>  fail:
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index 0a60deb..461b4a4 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -1342,6 +1342,18 @@ int tcp_mss_to_mtu(struct sock *sk, int mss)
>  	return mtu;
>  }
>  
> +static void icsk_mtup_probe_timer(unsigned long arg)
> +{
> +	struct sock *sk = (struct sock *)arg;
> +	struct net *net = sock_net(sk);
> +	struct inet_connection_sock *icsk = inet_csk(sk);
> +
> +	/* Restore orignal search range */
> +	icsk->icsk_mtup.search_high = icsk->icsk_mtup.search_high_sav;
> +	icsk->icsk_mtup.search_low = icsk->icsk_mtup.search_low_sav;
> +	icsk->icsk_mtup.probe_size = 0;
> +}
> +

As icsk_mtup_probe_timer() is run asynchronously, we may touch an invalid socket
instance if we don't hold socket's refcount before launching the timer.

Therefore, in general we use the standard interfaces like sk_reset_timer() and
sk_stop_timer() to operate timers associated with socket. So, the usage about
timer in the patch seems unsafe for us. For instance, you can study how
icsk_retransmit_timer, icsk_delack_timer and sk_timer, are implemented.

Regards,
Ying

>  /* MTU probing init per socket */
>  void tcp_mtup_init(struct sock *sk)
>  {
> @@ -1357,6 +1369,9 @@ void tcp_mtup_init(struct sock *sk)
>  	icsk->icsk_mtup.search_high_sav = icsk->icsk_mtup.search_high;
>  	icsk->icsk_mtup.search_low_sav = icsk->icsk_mtup.search_low;
>  	icsk->icsk_mtup.probe_size = 0;
> +
> +	setup_timer(&icsk->icsk_mtup.probe_timer, icsk_mtup_probe_timer,
> +		    (unsigned long)sk);
>  }
>  EXPORT_SYMBOL(tcp_mtup_init);
>  
> @@ -1840,6 +1855,7 @@ static int tcp_mtu_probe(struct sock *sk)
>  	struct tcp_sock *tp = tcp_sk(sk);
>  	struct inet_connection_sock *icsk = inet_csk(sk);
>  	struct sk_buff *skb, *nskb, *next;
> +	struct net *net = sock_net(sk);
>  	int len;
>  	int probe_size;
>  	int size_needed;
> @@ -1865,7 +1881,12 @@ static int tcp_mtu_probe(struct sock *sk)
>  	probe_size = (icsk->icsk_mtup.search_high + icsk->icsk_mtup.search_low) >> 1;
>  	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 */
> +		u32 probe_interval = net->ipv4.sysctl_tcp_probe_interval;
> +
> +		/* Search has been converged, start the timer,
> +		 * take advantage of path changing */
> +		mod_timer(&icsk->icsk_mtup.probe_timer,
> +			  jiffies + msecs_to_jiffies(1000*probe_interval));
>  		return -1;
>  	}
>  
> 

^ 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