Netdev List
 help / color / mirror / Atom feed
* RE: [RFC Patch net-next] tcp: add a global sysctl to control TCP delayed ack
From: David Laight @ 2013-01-16 12:22 UTC (permalink / raw)
  To: Cong Wang, netdev
  Cc: Eric Dumazet, Rick Jones, Stephen Hemminger, David S. Miller,
	Thomas Graf
In-Reply-To: <1358334345-28980-1-git-send-email-amwang@redhat.com>

> According to previous discussion, it seems there is no
> reasonable heuristics.
> 
> Similar to TCP_QUICK_ACK option, but for people who can't
> modify the source code and still wants to control
> TCP delayed ACK behavior.
> 
> Makes any sense?

A sysctl is a bit of a big hammer, it probably isn't necessary
to disable delayed acks on all connections.

IIRC the related problems I saw were really on the sending
side when Nagle is disabled and it is doing 'slow start'.

Globally disabling on connections that have Nagle disabled
might be a possibility - but it is the Nagle parameter
at the other end that matters.

Perhaps the sending side, after sending 4 small frames immediately,
could send 1 or 2 additional full sized frames in order to
provoke an ack (IIRC an ack is sent if there are 2 full sized
frames of data unacked).

The other problem is that 'slow start' is restarted very
aggressively - whenever there is no unacked data.
If you have a very low latency connection and aren't doing
continuous bulk transfer it is restarted for every short
burst of transmits - effectively after every received ack.
There really ought to have to be a moderate idle time
before 'slow start' is restarted.

	David

^ permalink raw reply

* Re: [RFC(v2) net-next 08/13] ipv6: Do not deoend on rt->n in ip6_dst_lookup_tail().
From: YOSHIFUJI Hideaki @ 2013-01-16 12:27 UTC (permalink / raw)
  To: Cong Wang; +Cc: netdev
In-Reply-To: <50F64D96.4090709@gmail.com>

Cong Wang wrote:
> s/deoend/depend/ in $subject...
> 
> On 01/16/2013 12:46 AM, YOSHIFUJI Hideaki wrote:
>> Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
>> ---
>>   net/ipv6/ip6_output.c |    8 ++++++--
>>   1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
>> index 9fc5d1d..b2fe048 100644
>> --- a/net/ipv6/ip6_output.c
>> +++ b/net/ipv6/ip6_output.c
>> @@ -921,8 +921,12 @@ static int ip6_dst_lookup_tail(struct sock *sk,
>>   	 * dst entry of the nexthop router
>>   	 */
>>   	rt = (struct rt6_info *) *dst;
>> -	n = rt->n;
>> -	if (n && !(n->nud_state & NUD_VALID)) {
>> +	rcu_read_lock_bh();
>> +	n = __ipv6_neigh_lookup_noref(rt->dst.dev, rt6_nexthop(rt, &fl6->daddr));
>> +	err = n && !(n->nud_state & NUD_VALID) ? -EINVAL : 0;
>> +	rcu_read_unlock_bh();
>> +
>> +	if (err) {
> 
> Are you sure the logic here equals?
> 
> In the original code, we could return 0 even we enter this if branch,
> but after your patch, it seems it will return -EINVAL?
> 

err is always updated after the branch, or return 0;

--yoshfuji

^ permalink raw reply

* [patch net-next] doc: add nf_conntrack sysctl api documentation
From: Jiri Pirko @ 2013-01-16 12:44 UTC (permalink / raw)
  To: netdev
  Cc: davem, rob, linux-doc, kuznet, jmorris, yoshfuji, pablo,
	netfilter-devel, netfilter, coreteam

I grepped through the code and picked bits about nf_conntrack sysctl api
and put that into one documentation file.

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 Documentation/networking/nf_conntrack-sysctl.txt | 160 +++++++++++++++++++++++
 1 file changed, 160 insertions(+)
 create mode 100644 Documentation/networking/nf_conntrack-sysctl.txt

diff --git a/Documentation/networking/nf_conntrack-sysctl.txt b/Documentation/networking/nf_conntrack-sysctl.txt
new file mode 100644
index 0000000..ab5f977
--- /dev/null
+++ b/Documentation/networking/nf_conntrack-sysctl.txt
@@ -0,0 +1,160 @@
+/proc/sys/net/netfilter/nf_conntrack_* Variables:
+
+nf_conntrack_acct - BOOLEAN
+	0 - disabled (default)
+	not 0 - enabled
+
+	Enable connection tracking flow accounting.
+
+nf_conntrack_buckets - INTEGER (read-only)
+	Size of hash table. Value is computed in nf_conntrack_init_init_net()
+	and it basically depends on total memory size.
+
+nf_conntrack_checksum - BOOLEAN
+	0 - disabled
+	not 0 - enabled (default)
+
+	Enable connection tracking checksuming.
+
+nf_conntrack_count - INTEGER (read-only)
+	Number of currently allocated conntracks.
+
+nf_conntrack_events - BOOLEAN
+	0 - disabled
+	not 0 - enabled (default)
+
+	If this option is enabled, the connection tracking code will provide
+	a notifier chain that can be used by other kernel code to get notified
+	about changes in the connection tracking state.
+
+nf_conntrack_events_retry_timeout - INTEGER (seconds)
+	default 15
+
+	Timeout after which destroy event will be delivered.
+
+nf_conntrack_expect_max - INTEGER
+	Maximum size of expectation table. Default value is computed in
+	nf_conntrack_expect_init() and depends on nf_conntrack_buckets value.
+
+nf_conntrack_frag6_high_thresh - INTEGER
+	default 262144
+
+	Maximum memory used to reassemble IPv6 fragments. When
+	nf_conntrack_frag6_high_thresh bytes of memory is allocated for this
+	purpose, the fragment handler will toss packets until
+	nf_conntrack_frag6_low_thresh is reached.
+
+nf_conntrack_frag6_low_thresh - INTEGER
+	default 196608
+
+	See nf_conntrack_frag6_low_thresh
+
+nf_conntrack_frag6_timeout - INTEGER (seconds)
+	default 60
+
+	Time to keep an IPv6 fragment in memory.
+
+nf_conntrack_generic_timeout - INTEGER (seconds)
+	default 600
+
+	Default for generic timeout.
+
+nf_conntrack_helper - BOOLEAN
+	0 - disabled
+	not 0 - enabled (default)
+
+	Enable automatic conntrack helper assignment.
+
+nf_conntrack_icmp_timeout - INTEGER (seconds)
+	default 30
+
+	Default for ICMP timeout.
+
+nf_conntrack_icmpv6_timeout - INTEGER (seconds)
+	default 30
+
+	Default for ICMP6 timeout.
+
+nf_conntrack_log_invalid - INTEGER
+	0 - disabled (default)
+	IPPROTO_RAW (log packets of any proto)
+	IPPROTO_TCP
+	IPPROTO_ICMP
+	IPPROTO_ICMPV6
+	IPPROTO_DCCP
+	IPPROTO_UDP
+	IPPROTO_UDPLITE
+
+	For values, see <linux/in.h>
+
+	Log invalid packets of a type specified by value.
+
+nf_conntrack_max - INTEGER
+	Size of connection tracking table. Default value is computed in
+	nf_conntrack_init_init_net() and depends on nf_conntrack_buckets value.
+
+nf_conntrack_tcp_be_liberal - BOOLEAN
+	0 - disabled (default)
+	not 0 - enabled
+
+	Be conservative in what you do, be liberal in what you accept from others.
+	If it's non-zero, we mark only out of window RST segments as INVALID.
+
+nf_conntrack_tcp_loose - BOOLEAN
+	0 - disabled
+	not 0 - enabled (default)
+
+	If it is set to zero, we disable picking up already established
+	connections.
+
+nf_conntrack_tcp_max_retrans - INTEGER
+	default 3
+
+	Max number of the retransmitted packets without receiving an
+	(acceptable) ACK from the destination. If this number is reached,
+	a shorter timer will be started.
+
+nf_conntrack_tcp_timeout_close - INTEGER (seconds)
+	default 10
+
+nf_conntrack_tcp_timeout_close_wait - INTEGER (seconds)
+	default 60
+
+nf_conntrack_tcp_timeout_established - INTEGER (seconds)
+	default 432000 (5 days)
+
+nf_conntrack_tcp_timeout_fin_wait - INTEGER (seconds)
+	default 120
+
+nf_conntrack_tcp_timeout_last_ack - INTEGER (seconds)
+	default 30
+
+nf_conntrack_tcp_timeout_max_retrans - INTEGER (seconds)
+	default 300
+
+nf_conntrack_tcp_timeout_syn_recv - INTEGER (seconds)
+	default 60
+
+nf_conntrack_tcp_timeout_syn_sent - INTEGER (seconds)
+	default 120
+
+nf_conntrack_tcp_timeout_time_wait - INTEGER (seconds)
+	default 120
+
+nf_conntrack_tcp_timeout_unacknowledged - INTEGER (seconds)
+	default 300
+
+nf_conntrack_timestamp - BOOLEAN
+	0 - disabled (default)
+	not 0 - enabled
+
+	Enable connection tracking flow timestamping.
+
+nf_conntrack_udp_timeout - INTEGER (seconds)
+	default 30
+
+nf_conntrack_udp_timeout_stream2 - INTEGER (seconds)
+	default 180
+
+	This extended timeout will be used in case there is an UDP stream
+	detected.
-- 
1.8.1


^ permalink raw reply related

* Re: [PATCH 2/2] e1000e: display a warning message when SmartSpeed works
From: Bjørn Mork @ 2013-01-16 12:59 UTC (permalink / raw)
  To: Koki Sanagi
  Cc: netdev, davem, jeffrey.t.kirsher, jesse.brandeburg, bruce.w.allan,
	carolyn.wyborny, donald.c.skidmore, gregory.v.rose,
	peter.p.waskiewicz.jr, alexander.h.duyck, john.ronciak,
	tushar.n.dave, e1000-devel
In-Reply-To: <50F60F18.2030108@jp.fujitsu.com>

Koki Sanagi <sanagi.koki@jp.fujitsu.com> writes:

> +			if (phy->speed_downgraded)
> +				netdev_warn(netdev, "Link Speed was "
> +					"downgraded by SmartSpeed\n");
> +

Could you please avoid breaking the string both here and in the igb
patch?  This breaks the ability to grep for the warning.

Quoting Documentation/CodingStyle, Chapter 2:
"However, never break user-visible strings such as printk messages,
 because that breaks the ability to grep for them."


Bjørn

^ permalink raw reply

* Re: [patch net-next] doc: add nf_conntrack sysctl api documentation
From: Florian Westphal @ 2013-01-16 13:26 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: netdev, davem, rob, linux-doc, kuznet, jmorris, yoshfuji, pablo,
	netfilter-devel, netfilter, coreteam
In-Reply-To: <1358340257-1902-1-git-send-email-jiri@resnulli.us>

Jiri Pirko <jiri@resnulli.us> wrote:
> I grepped through the code and picked bits about nf_conntrack sysctl api
> and put that into one documentation file.

Thanks a lot for doing this.  A few comments/suggestions below.

> +nf_conntrack_checksum - BOOLEAN
> +	0 - disabled
> +	not 0 - enabled (default)
> +
> +	Enable connection tracking checksuming.

Verify checksum of incoming packets.  Packets with bad checksum
will not be considered for connection tracking, i.e. such packets
will be in INVALID state.

> +nf_conntrack_events - BOOLEAN
> +	0 - disabled
> +	not 0 - enabled (default)
> +
> +	If this option is enabled, the connection tracking code will provide
> +	a notifier chain that can be used by other kernel code to get notified
> +	about changes in the connection tracking state.

If this option is enabled, the connection tracking code will
provide userspace with connection tracking events via ctnetlink.

[ The notifier call chain doesn't exist any more (ctnetlink was
the only user). ]

> +nf_conntrack_events_retry_timeout - INTEGER (seconds)
> +	default 15
> +
> +	Timeout after which destroy event will be delivered.

This option is only relevant when "reliable connection tracking
events" are used.  Normally, ctnetlink is "lossy", i.e. when
userspace listeners can't keep up, events are dropped.

Userspace can request "reliable event mode".  When this mode is
active, the conntrack will only be destroyed after the event was
delivered.  If event delivery fails, the kernel periodically
re-tries to send the event to userspace.

This is the maximum interval the kernel should use when re-trying
to deliver the destroy event.

Higher number means less delivery re-tries (but it will then take
longer for a backlog to be processed).

> +nf_conntrack_log_invalid - INTEGER
> +	0 - disabled (default)
> +	IPPROTO_RAW (log packets of any proto)
> +	IPPROTO_TCP
> +	IPPROTO_ICMP
> +	IPPROTO_ICMPV6
> +	IPPROTO_DCCP
> +	IPPROTO_UDP
> +	IPPROTO_UDPLITE
> +
> +	For values, see <linux/in.h>
> +
> +	Log invalid packets of a type specified by value.

I would write the numbers here, e.g:

Log invalid packets of a type specified by protocol number.
255 - log packets of any protocol
6 - log tcp
...

^ permalink raw reply

* [PATCH] ipv6: do not create neighbor entries for local delivery
From: Marcelo Ricardo Leitner @ 2013-01-16 13:31 UTC (permalink / raw)
  To: netdev

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

They will be created at output, if ever needed. This avoids creating
empty neighbor entries when TPROXYing/Forwarding packets for addresses
that are not even directly reachable.

Note that IPv4 already handles it this way. No neighbor entries are
created for local input.

Tested by myself and customer.

Signed-off-By: Marcelo Ricardo Leitner <mleitner@redhat.com>
---
  net/ipv6/route.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 
7c34c01b515b7ac370f8021ba6f5ec4c654e9e41..b63e33bcbdfbc1bd78af0b98ffc2d211805214ea 
100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -921,7 +921,7 @@ restart:
  	dst_hold(&rt->dst);
  	read_unlock_bh(&table->tb6_lock);
-	if (!rt->n && !(rt->rt6i_flags & RTF_NONEXTHOP))
+	if (!rt->n && !(rt->rt6i_flags & (RTF_NONEXTHOP|RTF_LOCAL)))
  		nrt = rt6_alloc_cow(rt, &fl6->daddr, &fl6->saddr);
  	else if (!(rt->dst.flags & DST_HOST))
  		nrt = rt6_alloc_clone(rt, &fl6->daddr);
-- 
1.7.11.7


[-- Attachment #2: Seção da mensagem anexada --]
[-- Type: text/plain, Size: 0 bytes --]



^ permalink raw reply

* Re: [PATCH V5 3/3] tuntap: allow polling/writing/reading when detached
From: Jason Wang @ 2013-01-16 13:35 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: davem, netdev, linux-kernel
In-Reply-To: <20130116105911.GA13470@redhat.com>

On 01/16/2013 06:59 PM, Michael S. Tsirkin wrote:
> On Wed, Jan 16, 2013 at 06:34:01PM +0800, Jason Wang wrote:
>> We forbid polling, writing and reading when the file were detached, this may
>> complex the user in several cases:
>>
>> - when guest pass some buffers to vhost/qemu and then disable some queues,
>>   host/qemu needs to do its own cleanup on those buffers which is complex
>>   sometimes. We can do this simply by allowing a user can still write to an
>>   disabled queue. Write to an disabled queue will cause the packet pass to the
>>   kernel and read will get nothing.
>> - align the polling behavior with macvtap which never fails when the queue is
>>   created. This can simplify the polling errors handling of its user (e.g vhost)
>>
>> In order to achieve this, tfile->tun were not assign to NULL when detached. And
>> tfile->tun were converted to be RCU protected in order to let the data path can
>> check whether the file is deated in a lockless manner. This will be used to
>> prevent the flow caches from being updated for a detached queue.
>>
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>
> NAK
>
>> ---
>>  drivers/net/tun.c |   43 +++++++++++++++++++++++++------------------
>>  1 files changed, 25 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
>> index c81680d..3f011e0 100644
>> --- a/drivers/net/tun.c
>> +++ b/drivers/net/tun.c
>> @@ -139,7 +139,7 @@ struct tun_file {
>>  	unsigned int flags;
>>  	u16 queue_index;
>>  	struct list_head next;
>> -	struct tun_struct *detached;
>> +	struct tun_struct __rcu *detached;
>>  };
>>  
>>  struct tun_flow_entry {
>> @@ -295,11 +295,12 @@ static void tun_flow_cleanup(unsigned long data)
>>  }
>>  
>>  static void tun_flow_update(struct tun_struct *tun, u32 rxhash,
>> -			    u16 queue_index)
>> +			    struct tun_file *tfile)
>>  {
>>  	struct hlist_head *head;
>>  	struct tun_flow_entry *e;
>>  	unsigned long delay = tun->ageing_time;
>> +	u16 queue_index = tfile->queue_index;
>>  
>>  	if (!rxhash)
>>  		return;
>> @@ -308,7 +309,7 @@ static void tun_flow_update(struct tun_struct *tun, u32 rxhash,
>>  
>>  	rcu_read_lock();
>>  
>> -	if (tun->numqueues == 1)
>> +	if (tun->numqueues == 1 || !rtnl_dereference(tfile->detached))
>>  		goto unlock;
>>  
>>  	e = tun_flow_find(head, rxhash);
> Did you try to run this with lockdep enabled?
> tun_flow_update is called from tun_get_user without rtnl so
> rtnl_dereference is arguably wrong, and will cause a lockdep warning.

Yes, will correct this and test with lockdep enabled.

Thanks
>
>> @@ -384,16 +385,16 @@ static void tun_set_real_num_queues(struct tun_struct *tun)
>>  
>>  static void tun_disable_queue(struct tun_struct *tun, struct tun_file *tfile)
>>  {
>> -	tfile->detached = tun;
>> +	rcu_assign_pointer(tfile->detached, tun);
>>  	list_add_tail(&tfile->next, &tun->disabled);
>>  	++tun->numdisabled;
>>  }
>>  
>>  static struct tun_struct *tun_enable_queue(struct tun_file *tfile)
>>  {
>> -	struct tun_struct *tun = tfile->detached;
>> +	struct tun_struct *tun = rtnl_dereference(tfile->detached);
>>  
>> -	tfile->detached = NULL;
>> +	rcu_assign_pointer(tfile->detached, NULL);
>>  	list_del_init(&tfile->next);
>>  	--tun->numdisabled;
>>  	return tun;
>> @@ -402,26 +403,27 @@ static struct tun_struct *tun_enable_queue(struct tun_file *tfile)
>>  static void __tun_detach(struct tun_file *tfile, bool clean)
>>  {
>>  	struct tun_file *ntfile;
>> -	struct tun_struct *tun;
>> +	struct tun_struct *tun, *detached;
>>  	struct net_device *dev;
>>  
>>  	tun = rtnl_dereference(tfile->tun);
>> +	detached = rtnl_dereference(tfile->detached);
>>  
>> -	if (tun) {
>> +	if (tun && !detached) {
>>  		u16 index = tfile->queue_index;
>>  		BUG_ON(index >= tun->numqueues);
>>  		dev = tun->dev;
>>  
>>  		rcu_assign_pointer(tun->tfiles[index],
>>  				   tun->tfiles[tun->numqueues - 1]);
>> -		rcu_assign_pointer(tfile->tun, NULL);
>>  		ntfile = rtnl_dereference(tun->tfiles[index]);
>>  		ntfile->queue_index = index;
>>  
>>  		--tun->numqueues;
>> -		if (clean)
>> +		if (clean) {
>> +			rcu_assign_pointer(tfile->tun, NULL);
>>  			sock_put(&tfile->sk);
>> -		else
>> +		} else
>>  			tun_disable_queue(tun, tfile);
>>  
>>  		synchronize_net();
>> @@ -429,7 +431,7 @@ static void __tun_detach(struct tun_file *tfile, bool clean)
>>  		/* Drop read queue */
>>  		skb_queue_purge(&tfile->sk.sk_receive_queue);
>>  		tun_set_real_num_queues(tun);
>> -	} else if (tfile->detached && clean) {
>> +	} else if (detached && clean) {
>>  		tun = tun_enable_queue(tfile);
>>  		sock_put(&tfile->sk);
>>  	}
>> @@ -466,6 +468,10 @@ static void tun_detach_all(struct net_device *dev)
>>  		rcu_assign_pointer(tfile->tun, NULL);
>>  		--tun->numqueues;
>>  	}
>> +	list_for_each_entry(tfile, &tun->disabled, next) {
>> +		wake_up_all(&tfile->wq.wait);
>> +		rcu_assign_pointer(tfile->tun, NULL);
>> +	}
>>  	BUG_ON(tun->numqueues != 0);
>>  
>>  	synchronize_net();
>> @@ -496,7 +502,7 @@ static int tun_attach(struct tun_struct *tun, struct file *file)
>>  		goto out;
>>  
>>  	err = -EINVAL;
>> -	if (rtnl_dereference(tfile->tun))
>> +	if (rtnl_dereference(tfile->tun) && !rtnl_dereference(tfile->detached))
>>  		goto out;
>>  
>>  	err = -EBUSY;
>> @@ -504,7 +510,7 @@ static int tun_attach(struct tun_struct *tun, struct file *file)
>>  		goto out;
>>  
>>  	err = -E2BIG;
>> -	if (!tfile->detached &&
>> +	if (!rtnl_dereference(tfile->detached) &&
>>  	    tun->numqueues + tun->numdisabled == MAX_TAP_QUEUES)
>>  		goto out;
>>  
>> @@ -521,7 +527,7 @@ static int tun_attach(struct tun_struct *tun, struct file *file)
>>  	rcu_assign_pointer(tun->tfiles[tun->numqueues], tfile);
>>  	tun->numqueues++;
>>  
>> -	if (tfile->detached)
>> +	if (rtnl_dereference(tfile->detached))
>>  		tun_enable_queue(tfile);
>>  	else
>>  		sock_hold(&tfile->sk);
>> @@ -1195,7 +1201,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
>>  	tun->dev->stats.rx_packets++;
>>  	tun->dev->stats.rx_bytes += len;
>>  
>> -	tun_flow_update(tun, rxhash, tfile->queue_index);
>> +	tun_flow_update(tun, rxhash, tfile);
>>  	return total_len;
>>  }
>>  
>> @@ -1796,7 +1802,7 @@ static int tun_set_queue(struct file *file, struct ifreq *ifr)
>>  	rtnl_lock();
>>  
>>  	if (ifr->ifr_flags & IFF_ATTACH_QUEUE) {
>> -		tun = tfile->detached;
>> +		tun = rtnl_dereference(tfile->detached);
>>  		if (!tun) {
>>  			ret = -EINVAL;
>>  			goto unlock;
>> @@ -1807,7 +1813,8 @@ static int tun_set_queue(struct file *file, struct ifreq *ifr)
>>  		ret = tun_attach(tun, file);
>>  	} else if (ifr->ifr_flags & IFF_DETACH_QUEUE) {
>>  		tun = rtnl_dereference(tfile->tun);
>> -		if (!tun || !(tun->flags & TUN_TAP_MQ))
>> +		if (!tun || !(tun->flags & TUN_TAP_MQ) ||
>> +		    rtnl_dereference(tfile->detached))
>>  			ret = -EINVAL;
>>  		else
>>  			__tun_detach(tfile, false);
>> -- 
>> 1.7.1
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" 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

* Re: [patch net-next] doc: add nf_conntrack sysctl api documentation
From: Jiri Pirko @ 2013-01-16 13:37 UTC (permalink / raw)
  To: Florian Westphal
  Cc: netdev, davem, rob, linux-doc, kuznet, jmorris, yoshfuji, pablo,
	netfilter-devel, netfilter, coreteam
In-Reply-To: <20130116132624.GD3484@breakpoint.cc>

Wed, Jan 16, 2013 at 02:26:24PM CET, fw@strlen.de wrote:
>Jiri Pirko <jiri@resnulli.us> wrote:
>> I grepped through the code and picked bits about nf_conntrack sysctl api
>> and put that into one documentation file.
>
>Thanks a lot for doing this.  A few comments/suggestions below.

Thanks for looking at this. I will process in your comments and send v2.

>
>> +nf_conntrack_checksum - BOOLEAN
>> +	0 - disabled
>> +	not 0 - enabled (default)
>> +
>> +	Enable connection tracking checksuming.
>
>Verify checksum of incoming packets.  Packets with bad checksum
>will not be considered for connection tracking, i.e. such packets
>will be in INVALID state.
>
>> +nf_conntrack_events - BOOLEAN
>> +	0 - disabled
>> +	not 0 - enabled (default)
>> +
>> +	If this option is enabled, the connection tracking code will provide
>> +	a notifier chain that can be used by other kernel code to get notified
>> +	about changes in the connection tracking state.
>
>If this option is enabled, the connection tracking code will
>provide userspace with connection tracking events via ctnetlink.
>
>[ The notifier call chain doesn't exist any more (ctnetlink was
>the only user). ]
>
>> +nf_conntrack_events_retry_timeout - INTEGER (seconds)
>> +	default 15
>> +
>> +	Timeout after which destroy event will be delivered.
>
>This option is only relevant when "reliable connection tracking
>events" are used.  Normally, ctnetlink is "lossy", i.e. when
>userspace listeners can't keep up, events are dropped.
>
>Userspace can request "reliable event mode".  When this mode is
>active, the conntrack will only be destroyed after the event was
>delivered.  If event delivery fails, the kernel periodically
>re-tries to send the event to userspace.
>
>This is the maximum interval the kernel should use when re-trying
>to deliver the destroy event.
>
>Higher number means less delivery re-tries (but it will then take
>longer for a backlog to be processed).
>
>> +nf_conntrack_log_invalid - INTEGER
>> +	0 - disabled (default)
>> +	IPPROTO_RAW (log packets of any proto)
>> +	IPPROTO_TCP
>> +	IPPROTO_ICMP
>> +	IPPROTO_ICMPV6
>> +	IPPROTO_DCCP
>> +	IPPROTO_UDP
>> +	IPPROTO_UDPLITE
>> +
>> +	For values, see <linux/in.h>
>> +
>> +	Log invalid packets of a type specified by value.
>
>I would write the numbers here, e.g:
>
>Log invalid packets of a type specified by protocol number.
>255 - log packets of any protocol
>6 - log tcp
>...

^ permalink raw reply

* Re: [Patch net-next 1/2] xfrm: replace rwlock on xfrm_state_afinfo with rcu
From: YOSHIFUJI Hideaki @ 2013-01-16 13:57 UTC (permalink / raw)
  To: Cong Wang
  Cc: netdev, Steffen Klassert, Herbert Xu, David S. Miller,
	YOSHIFUJI Hideaki
In-Reply-To: <1358323506-19571-1-git-send-email-amwang@redhat.com>

Cong Wang wrote:
> From: Cong Wang <amwang@redhat.com>
> 
> Similar to commit 418a99ac6ad487dc9c42e6b0e85f941af56330f2
> (Replace rwlock on xfrm_policy_afinfo with rcu), the rwlock
> on xfrm_state_afinfo can be replaced by RCU too.
> 
> Cc: Steffen Klassert <steffen.klassert@secunet.com>
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: "David S. Miller" <davem@davemloft.net>
> Signed-off-by: Cong Wang <amwang@redhat.com>
> ---
>  net/xfrm/xfrm_state.c |   33 ++++++++++++++++-----------------
>  1 files changed, 16 insertions(+), 17 deletions(-)
> 
> diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
> index 3459692..f567716 100644
> --- a/net/xfrm/xfrm_state.c
> +++ b/net/xfrm/xfrm_state.c
:
> @@ -173,17 +173,16 @@ static struct xfrm_state_afinfo *xfrm_state_lock_afinfo(unsigned int family)
>  	struct xfrm_state_afinfo *afinfo;
>  	if (unlikely(family >= NPROTO))
>  		return NULL;
> -	write_lock_bh(&xfrm_state_afinfo_lock);
> +	spin_lock_bh(&xfrm_state_afinfo_lock);
>  	afinfo = xfrm_state_afinfo[family];
>  	if (unlikely(!afinfo))
> -		write_unlock_bh(&xfrm_state_afinfo_lock);
> +		spin_unlock_bh(&xfrm_state_afinfo_lock);
>  	return afinfo;
>  }
>  
>  static void xfrm_state_unlock_afinfo(struct xfrm_state_afinfo *afinfo)
> -	__releases(xfrm_state_afinfo_lock)
>  {
> -	write_unlock_bh(&xfrm_state_afinfo_lock);
> +	spin_unlock_bh(&xfrm_state_afinfo_lock);
>  }
>  
>  int xfrm_register_type(const struct xfrm_type *type, unsigned short family)

Why removing __releases() hint?

--yoshfuji

^ permalink raw reply

* Re: 802.1q HW filter spammage in 3.7.2+ kernels.
From: Ben Greear @ 2013-01-16 14:03 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, netdev
In-Reply-To: <1358316108.19956.82.camel@edumazet-glaptop>

On 01/15/2013 10:01 PM, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> On Tue, 2013-01-15 at 17:36 -0800, Ben Greear wrote:
>> My kernel logs are full of this (I have 2000 macvlans).
>
> Are you using 2000 macvlans on the same device ? Interesting...

Well, in this case, we have 1000 mac-vlans on each of 2 1G
Ethernet ports, but 2000 on a single device has been used
as well.

>
> I am wondering how multicast/broadcast messages don't generate a huge
> load and packet drops, since we clone packets for every macvlan, and
> queue then to netif_rx()
>

I have added ARP patches to do a random retry timer to help spread
out the ARP requests a bit, at least.  The system is still sluggish
at times, but it does function.  With a fixed-interval ARP timer, things
can get into very bad patterns.

http://patchwork.ozlabs.org/patch/9301/

My test cases often require SO_BINDTODEVICE to function with full features,
with one traffic-application per interface, so mac-vlans seems the easiest
way to scale.

> I guess you dont use IPv6 on these macvlans ?

I have...but many use cases use far fewer (virtual) interfaces.

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

^ permalink raw reply

* Re: [RFC(v2) net-next 13/13] ipv6: Complete neighbour entry removal from dst_entry.
From: YOSHIFUJI Hideaki @ 2013-01-16 14:04 UTC (permalink / raw)
  To: Cong Wang; +Cc: netdev, YOSHIFUJI Hideaki
In-Reply-To: <50F6521F.4020706@gmail.com>

Cong Wang wrote:
> On 01/16/2013 12:46 AM, YOSHIFUJI Hideaki wrote:
>> +#if 0
>>   		if (rt->n && rt->n->dev == dev) {
>>   			rt->n->dev = loopback_dev;
>>   			dev_hold(loopback_dev);
>>   			dev_put(dev);
>>   		}
>> +#endif
> 
> Why commenting this out instead of removing it? As rt->n is totally
> removed, how possible could we reuse this code in future?

I have confirmed that this dev_put()/dev_hold() operation are really
for neigh->dev, which has refcnt for dev, so I agree.

(This is why I need to revisit refcnt 

--yoshfuji

^ permalink raw reply

* Re: [RFC(v2) net-next 01/13] ndisc: Refer/Update neigh->updated with write lock.
From: YOSHIFUJI Hideaki @ 2013-01-16 14:04 UTC (permalink / raw)
  To: Cong Wang; +Cc: netdev, YOSHIFUJI Hideaki
In-Reply-To: <50F64A3C.9040605@gmail.com>

Cong Wang wrote:
> On 01/16/2013 12:44 AM, YOSHIFUJI Hideaki wrote:
>> Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
>> ---
>>   net/ipv6/route.c |   12 ++++++++----
>>   1 file changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
>> index 7c34c01..1341f68 100644
>> --- a/net/ipv6/route.c
>> +++ b/net/ipv6/route.c
>> @@ -499,22 +499,26 @@ static void rt6_probe(struct rt6_info *rt)
>>   	 * to no more than one per minute.
>>   	 */
>>   	neigh = rt ? rt->n : NULL;
>> -	if (!neigh || (neigh->nud_state & NUD_VALID))
>> +	if (!neigh)
>> +		return;
>> +	write_lock_bh(&neigh->lock);
>> +	if (neigh->nud_state & NUD_VALID) {
>> +		write_unlock_bh(&neigh->lock);
>>   		return;
>> -	read_lock_bh(&neigh->lock);
>> +	}
>>   	if (!(neigh->nud_state & NUD_VALID) &&
>>   	    time_after(jiffies, neigh->updated + rt->rt6i_idev->cnf.rtr_probe_interval)) {
>>   		struct in6_addr mcaddr;
>>   		struct in6_addr *target;
>>   
>>   		neigh->updated = jiffies;
>> -		read_unlock_bh(&neigh->lock);
>> +		write_unlock_bh(&neigh->lock);
> 
> This looks like a bug fix, which deserves a separated patch rather than
> in this rt->n removal series.

Because the series depends on this, it is included here so far.

--yoshfuji

^ permalink raw reply

* Re: [PATCH] {cfg,mac}80211.h: fix some kernel-doc warnings
From: Johannes Berg @ 2013-01-16 14:11 UTC (permalink / raw)
  To: Yacine Belkadi
  Cc: David S. Miller, linux-wireless, netdev, linux-doc, linux-kernel
In-Reply-To: <1357995254-2318-1-git-send-email-yacine.belkadi.1@gmail.com>

Hi,

I totally missed your email before.

> Warning(include/net/cfg80211.h:334): No description found for return value of       'cfg80211_get_chandef_type'
> 
> These warnings are only reported when scripts/kernel-doc runs in verbose mode.
> 
> Fix:
> In comments, use "Return:" to describe function return values.

I wasn't even aware of this, is that new? Anyway, applied, thanks.

johannes

^ permalink raw reply

* Re: Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h>
From: YOSHIFUJI Hideaki @ 2013-01-16 14:21 UTC (permalink / raw)
  To: Cong Wang
  Cc: Thomas Backlund, Eric Blake, netdev, linux-kernel, libvirt-list,
	tgraf, David Miller, libc-alpha, schwab, carlos,
	YOSHIFUJI Hideaki
In-Reply-To: <1358316366.14898.8.camel@cr0>

Cong Wang wrote:
> (Cc'ing some glibc developers...)
> 
> Hello,
> 
> In glibc source file inet/netinet/in.h and kernel source file
> include/uapi/linux/in6.h, both define struct in6_addr, and both are
> visible to user applications. Thomas reported a conflict below.
> 
> So, how can we handle this? /me is wondering why we didn't see this
> before.
> 
> Thanks.
> 
> On Tue, 2013-01-15 at 12:55 +0200, Thomas Backlund wrote:
>> Cong Wang skrev 15.1.2013 12:11:
>>>
>>> Does the following patch help?
>>>
>>> $ git diff include/uapi/linux/if_bridge.h
>>> diff --git a/include/uapi/linux/if_bridge.h
>>> b/include/uapi/linux/if_bridge.h
>>> index 5db2975..653db23 100644
>>> --- a/include/uapi/linux/if_bridge.h
>>> +++ b/include/uapi/linux/if_bridge.h
>>> @@ -14,6 +14,7 @@
>>>   #define _UAPI_LINUX_IF_BRIDGE_H
>>>
>>>   #include <linux/types.h>
>>> +#include <linux/in6.h>
>>>
>>>   #define SYSFS_BRIDGE_ATTR      "bridge"
>>>   #define SYSFS_BRIDGE_FDB       "brforward"
>>>
>>
>> Well, I suggested the same fix in the beginning of the thread
>> on netdev and lkml: "if_bridge.h: include in6.h for struct in6_addr use"
>>
>> as it seemed to fix the libvirt case
>>
>> but then asked it to be ignored after I tried to build connman,
>> and hit this conflict with glibc-2.17:
>>
>> In file included from /usr/include/arpa/inet.h:22:0,
>>                   from ./include/connman/inet.h:25,
>>                   from src/connman.h:128,
>>                   from src/tethering.c:40:
>> /usr/include/netinet/in.h:35:5: error: expected identifier before 
>> numeric constant
>> /usr/include/netinet/in.h:197:8: error: redefinition of 'struct in6_addr'
>> In file included from /usr/include/linux/if_bridge.h:17:0,
>>                   from src/tethering.c:38:
>> /usr/include/linux/in6.h:30:8: note: originally defined here
>> In file included from /usr/include/arpa/inet.h:22:0,
>>                   from ./include/connman/inet.h:25,
>>                   from src/connman.h:128,
>>                   from src/tethering.c:40:
>> /usr/include/netinet/in.h:238:8: error: redefinition of 'struct 
>> sockaddr_in6'
>> In file included from /usr/include/linux/if_bridge.h:17:0,
>>                   from src/tethering.c:38:
>> /usr/include/linux/in6.h:46:8: note: originally defined here
>> In file included from /usr/include/arpa/inet.h:22:0,
>>                   from ./include/connman/inet.h:25,
>>                   from src/connman.h:128,
>>                   from src/tethering.c:40:
>> /usr/include/netinet/in.h:274:8: error: redefinition of 'struct ipv6_mreq'
>> In file included from /usr/include/linux/if_bridge.h:17:0,
>>                   from src/tethering.c:38:
>> /usr/include/linux/in6.h:54:8: note: originally defined here
>> make[1]: *** [src/src_connmand-tethering.o] Error 1
>>
>>
>> So I'm not sure it's the right one...

This is not a new issue.  In addition to this,
netinet/in.h also conflits with linux/in.h.

We might have
 #if !defined(__GLIBC__) || !defined(_NETINET_IN_H)
 :
 #endif
around those conflicting definitions in uapi/linux/in{,6}.h.

--yoshfuji

^ permalink raw reply

* [PATCH v3 1/2] net: asix: init ASIX AX88772B MAC from EEPROM
From: Lucas Stach @ 2013-01-16 14:24 UTC (permalink / raw)
  To: netdev-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA, Oliver Neukum, Christian Riesch,
	eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w

The device comes up with a MAC address of all zeros. We need to read the
initial device MAC from EEPROM so it can be set properly later.

Signed-off-by: Lucas Stach <dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org>
---
A similar fix was added into U-Boot:
http://patchwork.ozlabs.org/patch/179409/

v2: pass flag in the data field instead of clobbering the global flags
    space
v3: no change
---
 drivers/net/usb/asix.h         |  3 +++
 drivers/net/usb/asix_devices.c | 30 +++++++++++++++++++++++++++---
 2 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/drivers/net/usb/asix.h b/drivers/net/usb/asix.h
index e889631..7afe8ac 100644
--- a/drivers/net/usb/asix.h
+++ b/drivers/net/usb/asix.h
@@ -167,6 +167,9 @@ struct asix_data {
 	u8 res;
 };
 
+/* ASIX specific flags */
+#define FLAG_EEPROM_MAC		(1UL << 0)  /* init device MAC from eeprom */
+
 int asix_read_cmd(struct usbnet *dev, u8 cmd, u16 value, u16 index,
 		  u16 size, void *data);
 
diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c
index 7a6e758..0ecc3bc 100644
--- a/drivers/net/usb/asix_devices.c
+++ b/drivers/net/usb/asix_devices.c
@@ -422,14 +422,25 @@ static const struct net_device_ops ax88772_netdev_ops = {
 
 static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf)
 {
-	int ret, embd_phy;
+	int ret, embd_phy, i;
 	u8 buf[ETH_ALEN];
 	u32 phyid;
 
 	usbnet_get_endpoints(dev,intf);
 
 	/* Get the MAC address */
-	ret = asix_read_cmd(dev, AX_CMD_READ_NODE_ID, 0, 0, ETH_ALEN, buf);
+	if (dev->driver_info->data & FLAG_EEPROM_MAC) {
+		for (i = 0; i < (ETH_ALEN >> 1); i++) {
+			ret = asix_read_cmd(dev, AX_CMD_READ_EEPROM, 0x04 + i,
+					0, 2, buf + i * 2);
+			if (ret < 0)
+				break;
+		}
+	} else {
+		ret = asix_read_cmd(dev, AX_CMD_READ_NODE_ID,
+				0, 0, ETH_ALEN, buf);
+	}
+
 	if (ret < 0) {
 		netdev_dbg(dev->net, "Failed to read MAC address: %d\n", ret);
 		return ret;
@@ -872,6 +883,19 @@ static const struct driver_info ax88772_info = {
 	.tx_fixup = asix_tx_fixup,
 };
 
+static const struct driver_info ax88772b_info = {
+	.description = "ASIX AX88772B USB 2.0 Ethernet",
+	.bind = ax88772_bind,
+	.status = asix_status,
+	.link_reset = ax88772_link_reset,
+	.reset = ax88772_reset,
+	.flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR |
+	         FLAG_MULTI_PACKET,
+	.rx_fixup = asix_rx_fixup,
+	.tx_fixup = asix_tx_fixup,
+	.data = FLAG_EEPROM_MAC,
+};
+
 static const struct driver_info ax88178_info = {
 	.description = "ASIX AX88178 USB 2.0 Ethernet",
 	.bind = ax88178_bind,
@@ -953,7 +977,7 @@ static const struct usb_device_id	products [] = {
 }, {
 	// ASIX AX88772B 10/100
 	USB_DEVICE (0x0b95, 0x772b),
-	.driver_info = (unsigned long) &ax88772_info,
+	.driver_info = (unsigned long) &ax88772b_info,
 }, {
 	// ASIX AX88772 10/100
 	USB_DEVICE (0x0b95, 0x7720),
-- 
1.8.0.2

--
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 v3 2/2] net: asix: handle packets crossing URB boundaries
From: Lucas Stach @ 2013-01-16 14:24 UTC (permalink / raw)
  To: netdev; +Cc: linux-usb, Oliver Neukum, Christian Riesch, eric.dumazet
In-Reply-To: <1358346247-10579-1-git-send-email-dev@lynxeye.de>

ASIX AX88772B started to pack data even more tightly. Packets and the ASIX packet
header may now cross URB boundaries. To handle this we have to introduce
some state between individual calls to asix_rx_fixup().

Signed-off-by: Lucas Stach <dev@lynxeye.de>
---
I've running this patch for some weeks already now and it gets rid of all
the commonly seen rx failures with AX88772B.

v2: don't forget to free driver_private
v3: don't break ax88172a. Generates a bit churn, but cleaner than the
    alternative of playing pointer casting tricks.
---
 drivers/net/usb/asix.h         | 15 ++++++-
 drivers/net/usb/asix_common.c  | 90 ++++++++++++++++++++++++++++++------------
 drivers/net/usb/asix_devices.c | 23 +++++++++--
 drivers/net/usb/ax88172a.c     | 11 +++++-
 4 files changed, 109 insertions(+), 30 deletions(-)

diff --git a/drivers/net/usb/asix.h b/drivers/net/usb/asix.h
index 7afe8ac..346c032 100644
--- a/drivers/net/usb/asix.h
+++ b/drivers/net/usb/asix.h
@@ -167,6 +167,17 @@ struct asix_data {
 	u8 res;
 };
 
+struct asix_rx_fixup_info {
+	struct sk_buff *ax_skb;
+	u32 header;
+	u16 size;
+	bool split_head;
+};
+
+struct asix_common_private {
+	struct asix_rx_fixup_info rx_fixup_info;
+};
+
 /* ASIX specific flags */
 #define FLAG_EEPROM_MAC		(1UL << 0)  /* init device MAC from eeprom */
 
@@ -179,7 +190,9 @@ int asix_write_cmd(struct usbnet *dev, u8 cmd, u16 value, u16 index,
 void asix_write_cmd_async(struct usbnet *dev, u8 cmd, u16 value,
 			  u16 index, u16 size, void *data);
 
-int asix_rx_fixup(struct usbnet *dev, struct sk_buff *skb);
+int asix_rx_fixup_internal(struct usbnet *dev, struct sk_buff *skb,
+			   struct asix_rx_fixup_info *rx);
+int asix_rx_fixup_common(struct usbnet *dev, struct sk_buff *skb);
 
 struct sk_buff *asix_tx_fixup(struct usbnet *dev, struct sk_buff *skb,
 			      gfp_t flags);
diff --git a/drivers/net/usb/asix_common.c b/drivers/net/usb/asix_common.c
index 50d1673..15450dc 100644
--- a/drivers/net/usb/asix_common.c
+++ b/drivers/net/usb/asix_common.c
@@ -51,49 +51,89 @@ void asix_write_cmd_async(struct usbnet *dev, u8 cmd, u16 value, u16 index,
 			       value, index, data, size);
 }
 
-int asix_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
+int asix_rx_fixup_internal(struct usbnet *dev, struct sk_buff *skb,
+			   struct asix_rx_fixup_info *rx)
 {
 	int offset = 0;
 
-	while (offset + sizeof(u32) < skb->len) {
-		struct sk_buff *ax_skb;
-		u16 size;
-		u32 header = get_unaligned_le32(skb->data + offset);
-
-		offset += sizeof(u32);
-
-		/* get the packet length */
-		size = (u16) (header & 0x7ff);
-		if (size != ((~header >> 16) & 0x07ff)) {
-			netdev_err(dev->net, "asix_rx_fixup() Bad Header Length\n");
-			return 0;
+	while (offset + sizeof(u16) <= skb->len) {
+		u16 remaining = 0;
+		unsigned char *data;
+
+		if (!rx->size) {
+			if ((skb->len - offset == sizeof(u16)) ||
+			    rx->split_head) {
+				if(!rx->split_head) {
+					rx->header = get_unaligned_le16(
+							skb->data + offset);
+					rx->split_head = true;
+					offset += sizeof(u16);
+					break;
+				} else {
+					rx->header |= (get_unaligned_le16(
+							skb->data + offset)
+							<< 16);
+					rx->split_head = false;
+					offset += sizeof(u16);
+				}
+			} else {
+				rx->header = get_unaligned_le32(skb->data +
+								offset);
+				offset += sizeof(u32);
+			}
+
+			/* get the packet length */
+			rx->size = (u16) (rx->header & 0x7ff);
+			if (rx->size != ((~rx->header >> 16) & 0x7ff)) {
+				netdev_err(dev->net, "asix_rx_fixup() Bad Header Length 0x%x, offset %d\n",
+					   rx->header, offset);
+				rx->size = 0;
+				return 0;
+			}
+			rx->ax_skb = netdev_alloc_skb_ip_align(dev->net,
+							       rx->size);
+			if (!rx->ax_skb)
+				return 0;
 		}
 
-		if ((size > dev->net->mtu + ETH_HLEN + VLAN_HLEN) ||
-		    (size + offset > skb->len)) {
+		if (rx->size > dev->net->mtu + ETH_HLEN + VLAN_HLEN) {
 			netdev_err(dev->net, "asix_rx_fixup() Bad RX Length %d\n",
-				   size);
+				   rx->size);
+			kfree_skb(rx->ax_skb);
 			return 0;
 		}
-		ax_skb = netdev_alloc_skb_ip_align(dev->net, size);
-		if (!ax_skb)
-			return 0;
 
-		skb_put(ax_skb, size);
-		memcpy(ax_skb->data, skb->data + offset, size);
-		usbnet_skb_return(dev, ax_skb);
+		if (rx->size > skb->len - offset) {
+			remaining = rx->size - (skb->len - offset);
+			rx->size = skb->len - offset;
+		}
+
+		data = skb_put(rx->ax_skb, rx->size);
+		memcpy(data, skb->data + offset, rx->size);
+		if (!remaining)
+			usbnet_skb_return(dev, rx->ax_skb);
 
-		offset += (size + 1) & 0xfffe;
+		offset += (rx->size + 1) & 0xfffe;
+		rx->size = remaining;
 	}
 
 	if (skb->len != offset) {
-		netdev_err(dev->net, "asix_rx_fixup() Bad SKB Length %d\n",
-			   skb->len);
+		netdev_err(dev->net, "asix_rx_fixup() Bad SKB Length %d, %d\n",
+			   skb->len, offset);
 		return 0;
 	}
+
 	return 1;
 }
 
+int asix_rx_fixup_common(struct usbnet *dev, struct sk_buff *skb)
+{
+	struct asix_common_private *dp = dev->driver_priv;
+	struct asix_rx_fixup_info *rx = &dp->rx_fixup_info;
+
+	return asix_rx_fixup_internal(dev, skb, rx);
+}
+
 struct sk_buff *asix_tx_fixup(struct usbnet *dev, struct sk_buff *skb,
 			      gfp_t flags)
 {
diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c
index 0ecc3bc..37de7db 100644
--- a/drivers/net/usb/asix_devices.c
+++ b/drivers/net/usb/asix_devices.c
@@ -495,9 +495,19 @@ static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf)
 		dev->rx_urb_size = 2048;
 	}
 
+	dev->driver_priv = kzalloc(sizeof(struct asix_common_private), GFP_KERNEL);
+	if (!dev->driver_priv)
+		return -ENOMEM;
+
 	return 0;
 }
 
+void ax88772_unbind(struct usbnet *dev, struct usb_interface *intf)
+{
+	if (dev->driver_priv)
+		kfree(dev->driver_priv);
+}
+
 static const struct ethtool_ops ax88178_ethtool_ops = {
 	.get_drvinfo		= asix_get_drvinfo,
 	.get_link		= asix_get_link,
@@ -829,6 +839,10 @@ static int ax88178_bind(struct usbnet *dev, struct usb_interface *intf)
 		dev->rx_urb_size = 2048;
 	}
 
+	dev->driver_priv = kzalloc(sizeof(struct asix_common_private), GFP_KERNEL);
+	if (!dev->driver_priv)
+			return -ENOMEM;
+
 	return 0;
 }
 
@@ -875,23 +889,25 @@ static const struct driver_info hawking_uf200_info = {
 static const struct driver_info ax88772_info = {
 	.description = "ASIX AX88772 USB 2.0 Ethernet",
 	.bind = ax88772_bind,
+	.unbind = ax88772_unbind,
 	.status = asix_status,
 	.link_reset = ax88772_link_reset,
 	.reset = ax88772_reset,
 	.flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR | FLAG_MULTI_PACKET,
-	.rx_fixup = asix_rx_fixup,
+	.rx_fixup = asix_rx_fixup_common,
 	.tx_fixup = asix_tx_fixup,
 };
 
 static const struct driver_info ax88772b_info = {
 	.description = "ASIX AX88772B USB 2.0 Ethernet",
 	.bind = ax88772_bind,
+	.unbind = ax88772_unbind,
 	.status = asix_status,
 	.link_reset = ax88772_link_reset,
 	.reset = ax88772_reset,
 	.flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR |
 	         FLAG_MULTI_PACKET,
-	.rx_fixup = asix_rx_fixup,
+	.rx_fixup = asix_rx_fixup_common,
 	.tx_fixup = asix_tx_fixup,
 	.data = FLAG_EEPROM_MAC,
 };
@@ -899,11 +915,12 @@ static const struct driver_info ax88772b_info = {
 static const struct driver_info ax88178_info = {
 	.description = "ASIX AX88178 USB 2.0 Ethernet",
 	.bind = ax88178_bind,
+	.unbind = ax88772_unbind,
 	.status = asix_status,
 	.link_reset = ax88178_link_reset,
 	.reset = ax88178_reset,
 	.flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR,
-	.rx_fixup = asix_rx_fixup,
+	.rx_fixup = asix_rx_fixup_common,
 	.tx_fixup = asix_tx_fixup,
 };
 
diff --git a/drivers/net/usb/ax88172a.c b/drivers/net/usb/ax88172a.c
index c8e0aa8..d535c1d 100644
--- a/drivers/net/usb/ax88172a.c
+++ b/drivers/net/usb/ax88172a.c
@@ -35,6 +35,7 @@ struct ax88172a_private {
 	u16 phy_addr;
 	u16 oldmode;
 	int use_embdphy;
+	struct asix_rx_fixup_info rx_fixup_info;
 };
 
 /* MDIO read and write wrappers for phylib */
@@ -400,6 +401,14 @@ out:
 
 }
 
+static int ax88172a_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
+{
+	struct ax88172a_private *dp = dev->driver_priv;
+	struct asix_rx_fixup_info *rx = &dp->rx_fixup_info;
+
+	return asix_rx_fixup_internal(dev, skb, rx);
+}
+
 const struct driver_info ax88172a_info = {
 	.description = "ASIX AX88172A USB 2.0 Ethernet",
 	.bind = ax88172a_bind,
@@ -409,6 +418,6 @@ const struct driver_info ax88172a_info = {
 	.status = ax88172a_status,
 	.flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR |
 		 FLAG_MULTI_PACKET,
-	.rx_fixup = asix_rx_fixup,
+	.rx_fixup = ax88172a_rx_fixup,
 	.tx_fixup = asix_tx_fixup,
 };
-- 
1.8.0.2

^ permalink raw reply related

* Re: [RFC(v2) net-next 12/13] ipv6: Remove temporary dst for icmpv6.
From: YOSHIFUJI Hideaki @ 2013-01-16 14:26 UTC (permalink / raw)
  To: Cong Wang; +Cc: netdev, YOSHIFUJI Hideaki
In-Reply-To: <50F6507B.5030908@gmail.com>

Cong Wang wrote:
> On 01/16/2013 12:46 AM, YOSHIFUJI Hideaki wrote:
>> Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
>> ---
>>   net/ipv6/ip6_fib.c |    2 +-
>>   net/ipv6/route.c   |   98 ----------------------------------------------------
>>   2 files changed, 1 insertion(+), 99 deletions(-)
> [...]
>> -
>> -int icmp6_dst_gc(void)
>> -{
> 
> Please remove its declaration too:
> include/net/ip6_route.h:extern int icmp6_dst_gc(void);

Agreed.  Also removed icmp6_dst_alloc().

--yoshfuji

^ permalink raw reply

* [patch net-next v2] doc: add nf_conntrack sysctl api documentation
From: Jiri Pirko @ 2013-01-16 14:56 UTC (permalink / raw)
  To: netdev
  Cc: davem, rob, linux-doc, kuznet, jmorris, yoshfuji, pablo,
	netfilter-devel, netfilter, coreteam, fw

I grepped through the code and picked bits about nf_conntrack sysctl api
and put that into one documentation file.

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---

v1->v2:
processed in changes suggested by Florian Westphal

 Documentation/networking/nf_conntrack-sysctl.txt | 172 +++++++++++++++++++++++
 1 file changed, 172 insertions(+)
 create mode 100644 Documentation/networking/nf_conntrack-sysctl.txt

diff --git a/Documentation/networking/nf_conntrack-sysctl.txt b/Documentation/networking/nf_conntrack-sysctl.txt
new file mode 100644
index 0000000..61b66e6
--- /dev/null
+++ b/Documentation/networking/nf_conntrack-sysctl.txt
@@ -0,0 +1,172 @@
+/proc/sys/net/netfilter/nf_conntrack_* Variables:
+
+nf_conntrack_acct - BOOLEAN
+	0 - disabled (default)
+	not 0 - enabled
+
+	Enable connection tracking flow accounting.
+
+nf_conntrack_buckets - INTEGER (read-only)
+	Size of hash table. Value is computed in nf_conntrack_init_init_net()
+	and it basically depends on total memory size.
+
+nf_conntrack_checksum - BOOLEAN
+	0 - disabled
+	not 0 - enabled (default)
+
+	Verify checksum of incoming packets.  Packets with bad checksum
+	will not be considered for connection tracking, i.e. such packets
+	will be in INVALID state.
+
+nf_conntrack_count - INTEGER (read-only)
+	Number of currently allocated conntracks.
+
+nf_conntrack_events - BOOLEAN
+	0 - disabled
+	not 0 - enabled (default)
+
+	If this option is enabled, the connection tracking code will
+	provide userspace with connection tracking events via ctnetlink.
+
+nf_conntrack_events_retry_timeout - INTEGER (seconds)
+	default 15
+
+	This option is only relevant when "reliable connection tracking
+	events" are used.  Normally, ctnetlink is "lossy", i.e. when
+	userspace listeners can't keep up, events are dropped.
+
+	Userspace can request "reliable event mode".  When this mode is
+	active, the conntrack will only be destroyed after the event was
+	delivered.  If event delivery fails, the kernel periodically
+	re-tries to send the event to userspace.
+
+	This is the maximum interval the kernel should use when re-trying
+	to deliver the destroy event.
+
+	Higher number means less delivery re-tries (but it will then take
+	longer for a backlog to be processed).
+
+nf_conntrack_expect_max - INTEGER
+	Maximum size of expectation table.  Default value is computed in
+	nf_conntrack_expect_init() and depends on nf_conntrack_buckets value.
+
+nf_conntrack_frag6_high_thresh - INTEGER
+	default 262144
+
+	Maximum memory used to reassemble IPv6 fragments.  When
+	nf_conntrack_frag6_high_thresh bytes of memory is allocated for this
+	purpose, the fragment handler will toss packets until
+	nf_conntrack_frag6_low_thresh is reached.
+
+nf_conntrack_frag6_low_thresh - INTEGER
+	default 196608
+
+	See nf_conntrack_frag6_low_thresh
+
+nf_conntrack_frag6_timeout - INTEGER (seconds)
+	default 60
+
+	Time to keep an IPv6 fragment in memory.
+
+nf_conntrack_generic_timeout - INTEGER (seconds)
+	default 600
+
+	Default for generic timeout.
+
+nf_conntrack_helper - BOOLEAN
+	0 - disabled
+	not 0 - enabled (default)
+
+	Enable automatic conntrack helper assignment.
+
+nf_conntrack_icmp_timeout - INTEGER (seconds)
+	default 30
+
+	Default for ICMP timeout.
+
+nf_conntrack_icmpv6_timeout - INTEGER (seconds)
+	default 30
+
+	Default for ICMP6 timeout.
+
+nf_conntrack_log_invalid - INTEGER
+	0   - disable (default)
+	1   - log ICMP packets
+	6   - log TCP packets
+	17  - log UDP packets
+	33  - log DCCP packets
+	41  - log ICMPv6 packets
+	136 - log UDPLITE packets
+	255 - log packets of any protocol
+
+	Log invalid packets of a type specified by value.
+
+nf_conntrack_max - INTEGER
+	Size of connection tracking table.  Default value is computed in
+	nf_conntrack_init_init_net() and depends on nf_conntrack_buckets value.
+
+nf_conntrack_tcp_be_liberal - BOOLEAN
+	0 - disabled (default)
+	not 0 - enabled
+
+	Be conservative in what you do, be liberal in what you accept from others.
+	If it's non-zero, we mark only out of window RST segments as INVALID.
+
+nf_conntrack_tcp_loose - BOOLEAN
+	0 - disabled
+	not 0 - enabled (default)
+
+	If it is set to zero, we disable picking up already established
+	connections.
+
+nf_conntrack_tcp_max_retrans - INTEGER
+	default 3
+
+	Max number of the retransmitted packets without receiving an
+	(acceptable) ACK from the destination.  If this number is reached,
+	a shorter timer will be started.
+
+nf_conntrack_tcp_timeout_close - INTEGER (seconds)
+	default 10
+
+nf_conntrack_tcp_timeout_close_wait - INTEGER (seconds)
+	default 60
+
+nf_conntrack_tcp_timeout_established - INTEGER (seconds)
+	default 432000 (5 days)
+
+nf_conntrack_tcp_timeout_fin_wait - INTEGER (seconds)
+	default 120
+
+nf_conntrack_tcp_timeout_last_ack - INTEGER (seconds)
+	default 30
+
+nf_conntrack_tcp_timeout_max_retrans - INTEGER (seconds)
+	default 300
+
+nf_conntrack_tcp_timeout_syn_recv - INTEGER (seconds)
+	default 60
+
+nf_conntrack_tcp_timeout_syn_sent - INTEGER (seconds)
+	default 120
+
+nf_conntrack_tcp_timeout_time_wait - INTEGER (seconds)
+	default 120
+
+nf_conntrack_tcp_timeout_unacknowledged - INTEGER (seconds)
+	default 300
+
+nf_conntrack_timestamp - BOOLEAN
+	0 - disabled (default)
+	not 0 - enabled
+
+	Enable connection tracking flow timestamping.
+
+nf_conntrack_udp_timeout - INTEGER (seconds)
+	default 30
+
+nf_conntrack_udp_timeout_stream2 - INTEGER (seconds)
+	default 180
+
+	This extended timeout will be used in case there is an UDP stream
+	detected.
-- 
1.8.1


^ permalink raw reply related

* Re: [PATCH net-next] bnx2x: fix GRO parameters
From: Eric Dumazet @ 2013-01-16 15:29 UTC (permalink / raw)
  To: Yuval Mintz; +Cc: David Miller, netdev, Eilon Greenstein, ariele
In-Reply-To: <50F65060.5000901@broadcom.com>

On Wed, 2013-01-16 at 09:01 +0200, Yuval Mintz wrote:
> > -static u16 bnx2x_set_lro_mss(struct bnx2x *bp, u16 parsing_flags,
> > -			     u16 len_on_bd)
> > +static void bnx2x_set_gro_params(struct sk_buff *skb, struct bnx2x *bp,
> > +				 u16 parsing_flags, u16 len_on_bd,
> > +				 unsigned int pkt_len)
> 
> This is purely semantic, but our convention is for `struct bnx2x' to be
> the first argument in our functions.
> 
> >  {
> >  	/*
> > -	 * TPA arrgregation won't have either IP options or TCP options
> > +	 * TPA aggregation won't have either IP options or TCP options
> >  	 * other than timestamp or IPv6 extension headers.
> >  	 */
> >  	u16 hdrs_len = ETH_HLEN + sizeof(struct tcphdr);
> 
> TPA_MODE_LRO indicates that an LRO aggregation was made by our FW. It seems
> like your patch  eliminates the difference in configuration between the two
> (GRO and LRO)
> 

Thats the case, since you call GRO functions even if 'LRO ' is ON

I specifically had to remove the tests you guys do.

> perhaps instead we should do something like:
> 
> + static void bnx2x_set_lro_params(struct bnx2x *bp, struct sk_buff *skb,
> + 				   u16 parsing_flags, u16 len_on_bd,
> + 				   unsigned int pkt_len,
> +				   bnx2x_tpa_mode_t mode)
> 
> And arrange its suggested code so that only gso_size will be set for LRO.
> 

I fail to understand why adding a conditional would change something.

Setting it is needed for GRO as well.

> >  
> >  	if (GET_FLAG(parsing_flags, PARSING_FLAGS_OVER_ETHERNET_PROTOCOL) ==
> > -	    PRS_FLAG_OVERETH_IPV6)
> > +	    PRS_FLAG_OVERETH_IPV6) {
> >  		hdrs_len += sizeof(struct ipv6hdr);
> > -	else /* IPv4 */
> > +		skb_shinfo(skb)->gso_type = SKB_GSO_TCPV6;
> > +	} else {
> >  		hdrs_len += sizeof(struct iphdr);
> > -
> > +		skb_shinfo(skb)->gso_type = SKB_GSO_TCPV4;
> > +	}
> >  
> 
> 
> 
> >  #ifdef BNX2X_STOP_ON_ERROR
> > @@ -651,7 +655,7 @@ static void bnx2x_gro_receive(struct bnx2x *bp, struct bnx2x_fastpath *fp,
> >  			       struct sk_buff *skb)
> >  {
> >  #ifdef CONFIG_INET
> > -	if (fp->mode == TPA_MODE_GRO && skb_shinfo(skb)->gso_size) {
> > +	if (skb_shinfo(skb)->gso_size) {
> 
> This also seems like an incorrect removal, as TPA_MODE_LRO is (again)
> a feasible option, and we wouldn't want the a `gro_complete' here.
> 


Problem is : You call GRO functions, faking a GRO packet.

You must therefore exactly present same attributes in skb than after a
true software GRO step.

I did my tests booting a standard driver, that is with LRO on.

^ permalink raw reply

* Re: [PATCH net-next] bnx2x: fix GRO parameters
From: Yuval Mintz @ 2013-01-16 14:38 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, netdev, Eilon Greenstein, ariele
In-Reply-To: <1358350146.19956.658.camel@edumazet-glaptop>

>>> -static u16 bnx2x_set_lro_mss(struct bnx2x *bp, u16 parsing_flags,
>>> -			     u16 len_on_bd)
>>> +static void bnx2x_set_gro_params(struct sk_buff *skb, struct bnx2x *bp,
>>> +				 u16 parsing_flags, u16 len_on_bd,
>>> +				 unsigned int pkt_len)
>>
>> This is purely semantic, but our convention is for `struct bnx2x' to be
>> the first argument in our functions.
>>
>>>  {
>>>  	/*
>>> -	 * TPA arrgregation won't have either IP options or TCP options
>>> +	 * TPA aggregation won't have either IP options or TCP options
>>>  	 * other than timestamp or IPv6 extension headers.
>>>  	 */
>>>  	u16 hdrs_len = ETH_HLEN + sizeof(struct tcphdr);
>>
>> TPA_MODE_LRO indicates that an LRO aggregation was made by our FW. It seems
>> like your patch  eliminates the difference in configuration between the two
>> (GRO and LRO)
>>
> 
> Thats the case, since you call GRO functions even if 'LRO ' is ON
> 
> I specifically had to remove the tests you guys do.
> 
>> perhaps instead we should do something like:
>>
>> + static void bnx2x_set_lro_params(struct bnx2x *bp, struct sk_buff *skb,
>> + 				   u16 parsing_flags, u16 len_on_bd,
>> + 				   unsigned int pkt_len,
>> +				   bnx2x_tpa_mode_t mode)
>>
>> And arrange its suggested code so that only gso_size will be set for LRO.
>>
> 
> I fail to understand why adding a conditional would change something.
> 
> Setting it is needed for GRO as well.
> 

I was a bit unclear - I meant the for LRO, only gso_size will be set
(while for GRO it will be set as well as the additional GRO fields)

>>>  
>>>  	if (GET_FLAG(parsing_flags, PARSING_FLAGS_OVER_ETHERNET_PROTOCOL) ==
>>> -	    PRS_FLAG_OVERETH_IPV6)
>>> +	    PRS_FLAG_OVERETH_IPV6) {
>>>  		hdrs_len += sizeof(struct ipv6hdr);
>>> -	else /* IPv4 */
>>> +		skb_shinfo(skb)->gso_type = SKB_GSO_TCPV6;
>>> +	} else {
>>>  		hdrs_len += sizeof(struct iphdr);
>>> -
>>> +		skb_shinfo(skb)->gso_type = SKB_GSO_TCPV4;
>>> +	}
>>>  
>>
>>
>>
>>>  #ifdef BNX2X_STOP_ON_ERROR
>>> @@ -651,7 +655,7 @@ static void bnx2x_gro_receive(struct bnx2x *bp, struct bnx2x_fastpath *fp,
>>>  			       struct sk_buff *skb)
>>>  {
>>>  #ifdef CONFIG_INET
>>> -	if (fp->mode == TPA_MODE_GRO && skb_shinfo(skb)->gso_size) {
>>> +	if (skb_shinfo(skb)->gso_size) {
>>
>> This also seems like an incorrect removal, as TPA_MODE_LRO is (again)
>> a feasible option, and we wouldn't want the a `gro_complete' here.
>>
> 
> 
> Problem is : You call GRO functions, faking a GRO packet.
> 
> You must therefore exactly present same attributes in skb than after a
> true software GRO step.
> 
> I did my tests booting a standard driver, that is with LRO on.

I think we have a misunderstanding here - when LRO is on, our FW works in
TPA_MODE_LRO (LRO/GRO FW are mutually exclusive). In that mode, the bnx2x
driver will not try to 'fake' GRO packets in the same manner.

^ permalink raw reply

* [PATCH net-next 1/7] net/mlx4_en: Issue the dump eth statistics command under lock
From: Amir Vadai @ 2013-01-16 15:42 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, Or Gerlitz, Amir Vadai, Eugenia Emantayev
In-Reply-To: <1358350935-3281-1-git-send-email-amirv@mellanox.com>

From: Eugenia Emantayev <eugenia@mellanox.com>

Performing the DUMP_ETH_STATS firmware command outside the lock leads to kernel
panic when data structures such as RX/TX rings are freed in parallel, e.g when
one changes the mtu or ring sizes.

Signed-off-by: Eugenia Emantayev <eugenia@mellanox.com>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/en_netdev.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index b467513..b6c645f 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -977,12 +977,12 @@ static void mlx4_en_do_get_stats(struct work_struct *work)
 	struct mlx4_en_dev *mdev = priv->mdev;
 	int err;
 
-	err = mlx4_en_DUMP_ETH_STATS(mdev, priv->port, 0);
-	if (err)
-		en_dbg(HW, priv, "Could not update stats\n");
-
 	mutex_lock(&mdev->state_lock);
 	if (mdev->device_up) {
+		err = mlx4_en_DUMP_ETH_STATS(mdev, priv->port, 0);
+		if (err)
+			en_dbg(HW, priv, "Could not update stats\n");
+
 		if (priv->port_up)
 			mlx4_en_auto_moderation(priv);
 
-- 
1.7.8.2

^ permalink raw reply related

* [PATCH net-next 3/7] net/mlx4_en: Use the correct netif lock on ndo_set_rx_mode
From: Amir Vadai @ 2013-01-16 15:42 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, Or Gerlitz, Amir Vadai, Eugenia Emantayev
In-Reply-To: <1358350935-3281-1-git-send-email-amirv@mellanox.com>

From: Eugenia Emantayev <eugenia@mellanox.com>

The device multicast list is protected by netif_addr_lock_bh in the networking core, we should
use this locking practice in mlx4_en too.

Signed-off-by: Eugenia Emantayev <eugenia@mellanox.com>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
Reviewed-by: Yevgeny Petrilin <yevgenyp@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/en_netdev.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index bab8cec..805e242 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -767,9 +767,9 @@ static void mlx4_en_do_set_multicast(struct work_struct *work)
 
 		/* Update multicast list - we cache all addresses so they won't
 		 * change while HW is updated holding the command semaphor */
-		netif_tx_lock_bh(dev);
+		netif_addr_lock_bh(dev);
 		mlx4_en_cache_mclist(dev);
-		netif_tx_unlock_bh(dev);
+		netif_addr_unlock_bh(dev);
 		list_for_each_entry(mclist, &priv->mc_list, list) {
 			mcast_addr = mlx4_en_mac_to_u64(mclist->addr);
 			mlx4_SET_MCAST_FLTR(mdev->dev, priv->port,
-- 
1.7.8.2

^ permalink raw reply related

* [PATCH net-next 2/7] net/mlx4_en: Fix traffic loss under promiscuous mode
From: Amir Vadai @ 2013-01-16 15:42 UTC (permalink / raw)
  To: David S. Miller
  Cc: netdev, Or Gerlitz, Amir Vadai, Aviad Yehezkel, Eugenia Emantayev,
	Hadar Hen Zion
In-Reply-To: <1358350935-3281-1-git-send-email-amirv@mellanox.com>

From: Aviad Yehezkel <aviadye@mellanox.com>

When port is stopped and flow steering mode is not device managed: promisc QP
rule wasn't removed from MCG table.
Added code to remove it in all flow steering modes.
In addition, promsic rule removal should be in stop port and not in start
port - moved it accordingly.

Signed-off-by: Aviad Yehezkel <aviadye@mellanox.com>
Signed-off-by: Eugenia Emantayev <eugenia@mellanox.com>
Signed-off-by: Hadar Hen Zion <hadarh@mellanox.com>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/en_netdev.c |   35 +++++++++++++++++------
 1 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index b6c645f..bab8cec 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -1167,15 +1167,6 @@ int mlx4_en_start_port(struct net_device *dev)
 
 	/* Must redo promiscuous mode setup. */
 	priv->flags &= ~(MLX4_EN_FLAG_PROMISC | MLX4_EN_FLAG_MC_PROMISC);
-	if (mdev->dev->caps.steering_mode ==
-	    MLX4_STEERING_MODE_DEVICE_MANAGED) {
-		mlx4_flow_steer_promisc_remove(mdev->dev,
-					       priv->port,
-					       MLX4_FS_PROMISC_UPLINK);
-		mlx4_flow_steer_promisc_remove(mdev->dev,
-					       priv->port,
-					       MLX4_FS_PROMISC_ALL_MULTI);
-	}
 
 	/* Schedule multicast task to populate multicast list */
 	queue_work(mdev->workqueue, &priv->mcast_task);
@@ -1227,6 +1218,32 @@ void mlx4_en_stop_port(struct net_device *dev)
 	/* Set port as not active */
 	priv->port_up = false;
 
+	/* Promsicuous mode */
+	if (mdev->dev->caps.steering_mode ==
+	    MLX4_STEERING_MODE_DEVICE_MANAGED) {
+		priv->flags &= ~(MLX4_EN_FLAG_PROMISC |
+				 MLX4_EN_FLAG_MC_PROMISC);
+		mlx4_flow_steer_promisc_remove(mdev->dev,
+					       priv->port,
+					       MLX4_FS_PROMISC_UPLINK);
+		mlx4_flow_steer_promisc_remove(mdev->dev,
+					       priv->port,
+					       MLX4_FS_PROMISC_ALL_MULTI);
+	} else if (priv->flags & MLX4_EN_FLAG_PROMISC) {
+		priv->flags &= ~MLX4_EN_FLAG_PROMISC;
+
+		/* Disable promiscouos mode */
+		mlx4_unicast_promisc_remove(mdev->dev, priv->base_qpn,
+					    priv->port);
+
+		/* Disable Multicast promisc */
+		if (priv->flags & MLX4_EN_FLAG_MC_PROMISC) {
+			mlx4_multicast_promisc_remove(mdev->dev, priv->base_qpn,
+						      priv->port);
+			priv->flags &= ~MLX4_EN_FLAG_MC_PROMISC;
+		}
+	}
+
 	/* Detach All multicasts */
 	memset(&mc_list[10], 0xff, ETH_ALEN);
 	mc_list[5] = priv->port; /* needed for B0 steering support */
-- 
1.7.8.2

^ permalink raw reply related

* [PATCH net-next 4/7] net/mlx4_en: Set carrier to off when a port is stopped
From: Amir Vadai @ 2013-01-16 15:42 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, Or Gerlitz, Amir Vadai, Eugenia Emantayev
In-Reply-To: <1358350935-3281-1-git-send-email-amirv@mellanox.com>

From: Eugenia Emantayev <eugenia@mellanox.com>

Under heavy CPU load changing ring size/mtu/etc. could result in transmit
timeout, since stop-start port might take more than 10 sec. Set
netif_carrier_off to prevent tx queue transmit timeout.

Signed-off-by: Eugenia Emantayev <eugenia@mellanox.com>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/en_netdev.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index 805e242..108c4cf 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -1212,6 +1212,7 @@ void mlx4_en_stop_port(struct net_device *dev)
 
 	/* Synchronize with tx routine */
 	netif_tx_lock_bh(dev);
+	netif_carrier_off(dev);
 	netif_tx_stop_all_queues(dev);
 	netif_tx_unlock_bh(dev);
 
-- 
1.7.8.2

^ permalink raw reply related

* [PATCH net-next 6/7] net/mlx4_en: Fix a race when closing TX queue
From: Amir Vadai @ 2013-01-16 15:42 UTC (permalink / raw)
  To: David S. Miller
  Cc: netdev, Or Gerlitz, Amir Vadai, Yevgeny Petrilin,
	Eugenia Emantayev
In-Reply-To: <1358350935-3281-1-git-send-email-amirv@mellanox.com>

There is a possible race where the TX completion handler can clean the
entire TX queue between the decision that the queue is full and actually
closing it. To avoid this situation, check again if the queue is really
full, if not, reopen the transmit and continue with sending the packet.

Signed-off-by: Yevgeny Petrilin <yevgenyp@mellanox.com>
Signed-off-by: Eugenia Emantayev <eugenia@mellanox.com>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/en_tx.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_tx.c b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
index 2b799f4..1d17f5f 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_tx.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
@@ -592,7 +592,14 @@ netdev_tx_t mlx4_en_xmit(struct sk_buff *skb, struct net_device *dev)
 		netif_tx_stop_queue(ring->tx_queue);
 		priv->port_stats.queue_stopped++;
 
-		return NETDEV_TX_BUSY;
+		/* Check again whether the queue was cleaned */
+		if (unlikely(((int)(ring->prod - ring->cons)) <=
+				ring->size - HEADROOM - MAX_DESC_TXBBS)) {
+			netif_tx_wake_queue(ring->tx_queue);
+			priv->port_stats.wake_queue++;
+		} else {
+			return NETDEV_TX_BUSY;
+		}
 	}
 
 	/* Track current inflight packets for performance analysis */
-- 
1.7.8.2

^ permalink raw reply related


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