Netdev List
 help / color / mirror / Atom feed
* [PATCH v2] xen-netback: allow changing the MAC address of the interface
From: Matt Wilson @ 2013-01-22 18:08 UTC (permalink / raw)
  To: linux-kernel
  Cc: Ian Campbell, Matt Wilson, netdev, xen-devel,
	Konrad Rzeszutek Wilk

Sometimes it is useful to be able to change the MAC address of the
interface for netback devices. For example, when using ebtables it may
be useful to be able to distinguish traffic from different interfaces
without depending on the interface name.

Reported-by: Nikita Borzykh <sample.n@gmail.com>
Reported-by: Paul Harvey <stockingpaul@hotmail.com>
Cc: netdev@vger.kernel.org
Cc: xen-devel@lists.xen.org
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Signed-off-by: Matt Wilson <msw@amazon.com>

---
Changes since v1:
 * No code changes, only updated commit message
---
 drivers/net/xen-netback/interface.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c
index b7d41f8..f733cae 100644
--- a/drivers/net/xen-netback/interface.c
+++ b/drivers/net/xen-netback/interface.c
@@ -238,6 +238,8 @@ static const struct net_device_ops xenvif_netdev_ops = {
 	.ndo_stop	= xenvif_close,
 	.ndo_change_mtu	= xenvif_change_mtu,
 	.ndo_fix_features = xenvif_fix_features,
+	.ndo_set_mac_address = eth_mac_addr,
+	.ndo_validate_addr   = eth_validate_addr,
 };
 
 struct xenvif *xenvif_alloc(struct device *parent, domid_t domid,
-- 
1.7.4.5

^ permalink raw reply related

* Re: [PATCH net-next V6 02/14] bridge: Add vlan filtering infrastructure
From: Vlad Yasevich @ 2013-01-22 17:32 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev@vger.kernel.org
In-Reply-To: <20130122091746.7a3820e9@nehalam.linuxnetplumber.net>

On 01/22/2013 12:17 PM, Stephen Hemminger wrote:
> I appreciate all the work on this. And at this point it may seem like it
> will never get in mainline. As far as I am concerned these are the key
> features:
>
>    1. VLAN filtering on both ingress and egress using same table

Can you elaborate a bit more what you mean by this?

>    2. O(1) based bit map table
>    3. netlink based configuration and dump (per port). Okay to just provide the
>      bitmap
>    4. kernel feature should be optional
>    5. default behavior has to be to allow all VLAN's for backwards compatibility.

I think with the exception of 2, all of the above requirements are met 
by the current code.  As for #2, a hash approach provides most of the 
performance gain and allows for extensibility.
I am currently removing the list from all the hot paths, and we could 
make the hash wider if you want to make each hlist shorter.

>
> Extra credit for:
>    * cleaning up kernel VLAN API's. Eliminate vlan_buggy() and any/all cases
>      where error is not detected until first packet

I don't think we can truly do that since non-vlans frames should work on 
vlan_buggy interfaces.

I could also remove all the HW filtering calls out of the code since 
they are not truly needed now due to running in promisc mode.

-vlad

>
> I don't care about:
>    * sysfs API's - sysfs is not good for binary (like bitmap), and doing
>      a list
>    * split ingress/egress
>    * future extensibility ideas - do it right now, then do the next step;
>      don't get ahead of yourself.
>

^ permalink raw reply

* Re: 3.7.3+:  Bad paging request in ip_rcv_finish while running NFS traffic.
From: Eric Dumazet @ 2013-01-22 17:26 UTC (permalink / raw)
  To: Ben Greear; +Cc: netdev, linux-nfs@vger.kernel.org
In-Reply-To: <1358875020.3464.4006.camel@edumazet-glaptop>

On Tue, 2013-01-22 at 09:17 -0800, Eric Dumazet wrote:
> On Tue, 2013-01-22 at 09:08 -0800, Ben Greear wrote:
> 
> > Unfortunately, I hit it again this morning after the first restart of
> > my application (which bounces all 3000 interfaces).  Memory poisoning
> > was disabled.
> 
> Is your NFS traffic using TCP or UDP ?
> 

Oh well, it seems macvlan.c has to skb_drop_dst(skb) before giving skb
to netif_rx()

diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 68a43fe..4f049ee 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -114,6 +114,7 @@ static int macvlan_broadcast_one(struct sk_buff *skb,
 	if (!skb)
 		return NET_RX_DROP;
 
+	skb_dst_drop(skb);
 	if (local)
 		return vlan->forward(dev, skb);
 
@@ -220,6 +221,7 @@ static rx_handler_result_t macvlan_handle_frame(struct sk_buff **pskb)
 	if (!skb)
 		goto out;
 
+	skb_dst_drop(skb);
 	skb->dev = dev;
 	skb->pkt_type = PACKET_HOST;
 

^ permalink raw reply related

* Re: 3.7.3+:  Bad paging request in ip_rcv_finish while running NFS traffic.
From: Ben Greear @ 2013-01-22 17:26 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev, linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <1358875020.3464.4006.camel@edumazet-glaptop>

On 01/22/2013 09:17 AM, Eric Dumazet wrote:
> On Tue, 2013-01-22 at 09:08 -0800, Ben Greear wrote:
>
>> Unfortunately, I hit it again this morning after the first restart of
>> my application (which bounces all 3000 interfaces).  Memory poisoning
>> was disabled.
>
> Is your NFS traffic using TCP or UDP ?

It's using TCP.

Thanks,
Ben

>


-- 
Ben Greear <greearb-my8/4N5VtI7c+919tysfdA@public.gmane.org>
Candela Technologies Inc  http://www.candelatech.com

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" 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

* Re: 3.7.3+:  Bad paging request in ip_rcv_finish while running NFS traffic.
From: Eric Dumazet @ 2013-01-22 17:17 UTC (permalink / raw)
  To: Ben Greear; +Cc: netdev, linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <50FEC796.5090404-my8/4N5VtI7c+919tysfdA@public.gmane.org>

On Tue, 2013-01-22 at 09:08 -0800, Ben Greear wrote:

> Unfortunately, I hit it again this morning after the first restart of
> my application (which bounces all 3000 interfaces).  Memory poisoning
> was disabled.

Is your NFS traffic using TCP or UDP ?


--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" 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

* Re: [PATCH net 2/3] net: cdc_mbim: send ZLP after max sized NTBs
From: Yauheni Kaliuta @ 2013-01-22 17:16 UTC (permalink / raw)
  To: Bjørn Mork
  Cc: netdev, linux-usb, Greg Suarez, Alexey Orishko, Oliver Neukum
In-Reply-To: <87hamab3fs.fsf@nemi.mork.no>

Hi!

On Tue, Jan 22, 2013 at 12:01 AM, Bjørn Mork <bjorn@mork.no> wrote:
> Yauheni Kaliuta <y.kaliuta@gmail.com> writes:
>>>>>>> "BM" == Bjørn Mork writes:
>>
>>  > We normally avoid sending ZLPs by padding NTBs with a zero byte
>>  > if the NTB is shorter than dwNtbOutMaxSize, resulting in a short
>>  > USB packet instead of a ZLP.  But in the case where the NTB length
>>  > is exactly dwNtbOutMaxSize and this is an exact multiplum of
>>  > wMaxPacketSize, then we must send a ZLP.
>>
>> The idea of NCM was to avoid extra ZLPs. If your transfer is exactly
>> dwNtbOutMaxSize, it's known, you can submit such request on the receiver
>> side and you do not need any EOT indicatation, so the frametime can be
>> used for useful data.
>
> Yes, that makes sense.  And I understand that both you and Alexey are of
> this opinion.
>
> But this idea is by no means made clear (to me) in the spec.  I do not
> think the current wording is precise enough to expect every implementor
> to understand any such intent.  The only place I find either "short
> packet" or ZLP mentioned in the NCM spec is in tables 3-1 and 3-2,
> describing the 16bit and 32bit NTH formats, in the description of the
> (d)wBlockLength fields.  And even there it is only mentioned in the
> context of the special (d)wBlockLength = 0x0000 handling.

I agree, it could be a bit unclear,

"
If exactly dwNtbInMaxSize or dwNtbOutMaxSize
bytes are sent, and the size is a multiple of wMax-
PacketSize for the given pipe, then no ZLP shall be
sent.
"
is an independent clause.

>
> If the intent was to prevent ZLPs, then it would have been wise to write
> that in the NCM standard. As it stands you have to use a lot of
> imagination to read that intent into the current spec.

Well, I hope the guys took your complains into account and will fix
the wording in the future spec versions.

^ permalink raw reply

* Re: Doubts about listen backlog and tcp_max_syn_backlog
From: Eric Dumazet @ 2013-01-22 17:13 UTC (permalink / raw)
  To: Leandro Lucarella; +Cc: netdev, linux-kernel
In-Reply-To: <20130122165929.GH4608@sociomantic.com>

On Tue, 2013-01-22 at 17:59 +0100, Leandro Lucarella wrote:

> What I'm seeing are clients taking either useconds to connect, or 3
> seconds, which suggest SYNs are getting lost, but the network doesn't
> seem to be the problem. I'm still investigating this, so unfortunately
> I'm not really sure.
> 

A SYN packet or a SYN-ACK packet can be lost in the network.

> > A serious rewrite of LISTEN code is needed, because the current
> > implementation doesn't scale :
> > 
> > The SYNACK retransmits are done by a single timer wheel, holding the
> > socket lock for too long. So increasing the backlog to 2^16 or 2^17 is
> > not really an option.
> > 
> > Hash table are nice, but if we have to scan them, holding a single lock,
> > they are not so nice.
> 
> So, the queue is really a hash table, then? So using any (2^n)-1 would
> be a bad idea because when the backlog is next to full, the hash table
> will be really slow? Is that why the + 1 is there? Is assuming everyone
> will use a power of 2 an thus having a load factor of 0.5 at most?
> 

The kind of hash tables we use are power of two.

The size of hash table has little effect, its automatic, to try to get
an average of one item per hash slot, or less. Even if we had 10 items
per slot, it would not be a big deal.

What is important is the backlog, and I guess you didn't increase it
properly. The somaxconn default is quite low (128)

# sysctl -w net/ipv4/tcp_max_syn_backlog=4096
net.ipv4.tcp_max_syn_backlog = 4096
# sysctl -w net.core.somaxconn=4096
net.core.somaxconn = 4096

Then make sure your server use a big enough listen(..., backlog)
parameter.

^ permalink raw reply

* Re: 3.7.3+:  Bad paging request in ip_rcv_finish while running NFS traffic.
From: Ben Greear @ 2013-01-22 17:08 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev, linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <50FE2A57.3040804-my8/4N5VtI7c+919tysfdA@public.gmane.org>

On 01/21/2013 09:57 PM, Ben Greear wrote:
> On 01/21/2013 08:40 PM, Eric Dumazet wrote:
>> On Mon, 2013-01-21 at 16:32 -0800, Ben Greear wrote:
>>> On 01/21/2013 01:07 PM, Ben Greear wrote:
>>>> I posted about this a few days ago, but this time the patches applied
>>>> are minimal and there are no out-of-tree kernel modules loaded.
>>>
>>> Here's another crash, this time with SLUB memory debugging turned on.
>>>
>>> Seems much harder to hit this way...I've only managed this one.  I
>>> believe the RCX register might be interesting...that 6b is probably
>>> freed memory poisioning.  Maybe skb or skb_dest() is already freed?
>>>
>>> I have added a 'verify_mem_not_deleted(skb)' before the
>>> dst_input line in ip_rcv_finish..but so far, it hasn't
>>> hit the problem...
>>>
>>
>> There is no way skb is freed here.
>>
>> I would say macvlan is at fault here.
>>
>> It probably lacks a proper (dev->flags & IFF_UP) test.
>
> Thanks for the patch.  It's running OK so far, but I'll need to
> do a bunch more testing tomorrow to make sure I'm not just getting
> lucky!
>
> I'll let you know how it goes tomorrow.

Unfortunately, I hit it again this morning after the first restart of
my application (which bounces all 3000 interfaces).  Memory poisoning
was disabled.

(gdb) l *(ip_rcv_finish+0x2b9)
0xffffffff814a8ab3 is in ip_rcv_finish (/home/greearb/git/linux-3.7.dev.y/net/ipv4/ip_input.c:373).
368					skb->len);
369		} else if (rt->rt_type == RTN_BROADCAST)
370			IP_UPD_PO_STATS_BH(dev_net(rt->dst.dev), IPSTATS_MIB_INBCAST,
371					skb->len);
372	
373		return dst_input(skb);
374	
375	drop:
376		kfree_skb(skb);
377		return NET_RX_DROP;
(gdb)

(gdb) l *(skb_dst+0x5a)
0xffffffff814a87fa is in ip_rcv_finish (/home/greearb/git/linux-3.7.dev.y/net/ipv4/ip_input.c:320).
315	
316	int sysctl_ip_early_demux __read_mostly = 1;
317	EXPORT_SYMBOL(sysctl_ip_early_demux);
318	
319	static int ip_rcv_finish(struct sk_buff *skb)
320	{
321		const struct iphdr *iph = ip_hdr(skb);
322		struct rtable *rt;
323	
324		if (sysctl_ip_early_demux && !skb_dst(skb)) {

nfs: server 10.1.0.1 not responding, timed out
nfs: server 10.1.0.1 not responding, timed out
BUG: unable to handle kernel NULL pointer dereference at           (null)
IP: [<          (null)>]           (null)
PGD 0
Oops: 0010 [#1] PREEMPT SMP
Modules linked in: nf_nat_ipv4 nf_nat nfsv4 auth_rpcgss nfs fscache 8021q garp stp llc lockd sunrpc macvlan pktgen uinput coretemp hwmon kvm_intel kvm iTCO_wdt 
iTCO_vendor_support gpio_ich microcode pcspkr i2c_i801 lpc_ich e1000e i7core_edac ioatdma edac_core igb ptp pps_core dca ipv6 mgag200 i2c_algo_bit 
drm_kms_helper ttm drm i2c_core [last unloaded: iptable_nat]
CPU 0
Pid: 9, comm: rcuc/0 Tainted: G         C O 3.7.3+ #43 Iron Systems Inc. EE2610R/X8ST3
RIP: 0010:[<0000000000000000>]  [<          (null)>]           (null)
RSP: 0018:ffff88041fc03da0  EFLAGS: 00010286
RAX: ffff88023045f5c0 RBX: ffff88034491ab00 RCX: 00000000ffff8800
RDX: ffff88025920d4fc RSI: ffffffff81a2a500 RDI: ffff88034491ab00
RBP: ffff88041fc03dc8 R08: ffffffff814a87fa R09: ffff88041fc03d90
R10: ffff88025920d4fc R11: ffff88041fc03e28 R12: ffff88025920d4fc
R13: ffff88041fc18b60 R14: ffff88040d3f8000 R15: 0000000000000000
FS:  0000000000000000(0000) GS:ffff88041fc00000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
CR2: 0000000000000000 CR3: 0000000001a0b000 CR4: 00000000000007f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
Process rcuc/0 (pid: 9, threadinfo ffff88040d4e2000, task ffff88040d4e8000)
Stack:
  ffffffff814a8ab3 ffff88034491ab00 ffffffff814a87fa ffff88034491ab00
  ffff88040d3f8000 ffff88041fc03df8 ffffffff814a8e66 0000000080000000
  ffffffff81472e61 ffff88034491ab00 ffff88040d3f8000 ffff88041fc03e28
Call Trace:
  <IRQ>
  [<ffffffff814a8ab3>] ? ip_rcv_finish+0x2b9/0x2d1
  [<ffffffff814a87fa>] ? skb_dst+0x5a/0x5a
  [<ffffffff814a8e66>] NF_HOOK.clone.1+0x4c/0x54
  [<ffffffff81472e61>] ? dev_seq_stop+0xb/0xb
  [<ffffffff814a90f3>] ip_rcv+0x237/0x268
  [<ffffffff81473def>] __netif_receive_skb+0x487/0x530
  [<ffffffff81473f91>] process_backlog+0xf9/0x1da
nfs: server 10.1.8.1 not responding, timed out
  [<ffffffff8147639a>] net_rx_action+0xad/0x218
  [<ffffffff8108d50a>] __do_softirq+0x9c/0x161
  [<ffffffff81538ddc>] call_softirq+0x1c/0x30
  <EOI>
  [<ffffffff8100bd21>] do_softirq+0x41/0x7e
  [<ffffffff8108d68b>] _local_bh_enable_ip+0x7a/0x9f
  [<ffffffff8108d6c8>] local_bh_enable+0xd/0x11
  [<ffffffff810f3661>] rcu_cpu_kthread+0xe6/0x11f
  [<ffffffff810a7ebe>] smpboot_thread_fn+0x253/0x259
  [<ffffffff810a7c6b>] ? test_ti_thread_flag.clone.0+0x11/0x11
  [<ffffffff810a0a6d>] kthread+0xc2/0xca
  [<ffffffff810a09ab>] ? __init_kthread_worker+0x56/0x56
  [<ffffffff81537afc>] ret_from_fork+0x7c/0xb0
  [<ffffffff810a09ab>] ? __init_kthread_worker+0x56/0x56
Code:  Bad RIP value.nfs: server 10.1.0.1 not responding, timed out
nfs: server 10.1.0.1 not responding, timed out

>
> Thanks,
> Ben
>
>
>


-- 
Ben Greear <greearb-my8/4N5VtI7c+919tysfdA@public.gmane.org>
Candela Technologies Inc  http://www.candelatech.com

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" 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

* Re: Doubts about listen backlog and tcp_max_syn_backlog
From: Leandro Lucarella @ 2013-01-22 16:59 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev, linux-kernel
In-Reply-To: <1358873142.3464.3964.camel@edumazet-glaptop>

On Tue, Jan 22, 2013 at 08:45:42AM -0800, Eric Dumazet wrote:
> On Tue, 2013-01-22 at 17:10 +0100, Leandro Lucarella wrote:
> > Hi, I'm having some problems with missing SYNs in a server with a high
> > rate of incoming connections and, even when far from understanding the
> > kernel,  I ended up looking at the kernel's source to try to understand
> > better what's going on, because some stuff doesn't make a lot of sense
> > to me.
[snip]
> > 1. What's the relation between the socket backlog and the queue created
> >    by reqsk_queue_alloc()? Because the backlog is only adjusted not to
> >    be grater than sysctl_somaxconn, but the queue size can be quite
> >    different.
> > 2. The comment just above the definition of reqsk_queue_alloc() about
> >    sysctl_max_syn_backlog says "Maximum number of SYN_RECV sockets in
> >    queue per LISTEN socket.". But then nr_table_entries is not only
> >    rounded up to the next power of 2, is incremented by one before that,
> >    so a backlog of, for example, 128, would end up with 256 table
> >    entries even if sysctl_max_syn_backlog is 128.
> > 3. Why is there a nr_table_entries + 1 at all in there? Looking at the
> >    commit that introduced this[1] I can't find any explanation and I've
> >    read some big projects are using backlogs of 511 because of this[2].
> >    (which BTW, ff the queue is really a hash table, looks like an awful
> >    idea).
> > 4. I found some places sk->sk_ack_backlog is checked against
> >    sk->sk_max_ack_backlog to see if new requests should be dropped, but
> >    I also saw checks like inet_csk_reqsk_queue_young(sk) > 1 or
> >    inet_csk_reqsk_queue_is_full(sk), so I guess the queue is used too.
[snip]
> 
> What particular problem do you have ?

What I'm seeing are clients taking either useconds to connect, or 3
seconds, which suggest SYNs are getting lost, but the network doesn't
seem to be the problem. I'm still investigating this, so unfortunately
I'm not really sure.

> A serious rewrite of LISTEN code is needed, because the current
> implementation doesn't scale :
> 
> The SYNACK retransmits are done by a single timer wheel, holding the
> socket lock for too long. So increasing the backlog to 2^16 or 2^17 is
> not really an option.
> 
> Hash table are nice, but if we have to scan them, holding a single lock,
> they are not so nice.

So, the queue is really a hash table, then? So using any (2^n)-1 would
be a bad idea because when the backlog is next to full, the hash table
will be really slow? Is that why the + 1 is there? Is assuming everyone
will use a power of 2 an thus having a load factor of 0.5 at most?

-- 
Leandro Lucarella
Senior R&D Developer
-----------------------------------------------------------
sociomantic labs GmbH
Paul-Lincke-Ufer 39/40
10999 Berlin
DEUTSCHLAND
-----------------------------------------------------------
http://www.sociomantic.com
-----------------------------------------------------------
Fon:       +49 (0) 30 3087 4615
Fax:       +49 (0) 30 3087 4619
Mobile:    +49 (0)157 3636 7373
Skype:     llucarella
Twitter:   http://www.twitter.com/sociomantic
Facebook:  http://bit.ly/labsfacebook
-----------------------------------------------------------
sociomantic labs GmbH, Location: Berlin
Commercial Register - AG Charlottenburg: HRB 121302 B
VAT No. - USt-ID: DE 266262100
Managing Directors: Thomas Nicolai, Thomas Brandhoff

^ permalink raw reply

* Re: BUG in netxen_release_tx_buffers when TSO enabled on kernels >= 3.3 and <= 3.6
From: Christoph Paasch @ 2013-01-22 16:55 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Ian Campbell, Sony Chacko, Rajesh Borundia, David Miller, netdev
In-Reply-To: <1358872385.3464.3940.camel@edumazet-glaptop>

On Tuesday 22 January 2013 08:33:05 Eric Dumazet wrote:
> [PATCH] netxen: fix off by one bug in netxen_release_tx_buffer()
> 
> Christoph Paasch found netxen could trigger a BUG in its dismantle
> phase, in netxen_release_tx_buffer(), using full size TSO packets.
> 
> cmd_buf->frag_count includes the skb->data part, so the loop must
> start at index 1 instead of 0, or else we can make an out
> of bound access to cmd_buff->frag_array[MAX_SKB_FRAGS + 2]
> 
> Christoph provided the fixes in netxen_map_tx_skb() function.
> In case of a dma mapping error, its better to clear the dma fields
> so that we don't try to unmap them again in netxen_release_tx_buffer()
> 
> Reported-by: Christoph Paasch <christoph.paasch@uclouvain.be>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Tested-by: Christoph Paasch <christoph.paasch@uclouvain.be>
> Cc: Sony Chacko <sony.chacko@qlogic.com>
> Cc: Rajesh Borundia <rajesh.borundia@qlogic.com>
> ---
>  drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c |    2 +-
>  drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c |    2 ++
>  2 files changed, 3 insertions(+), 1 deletion(-)

Signed-off-by: Christoph Paasch <christoph.paasch@uclouvain.be>


-- 
IP Networking Lab --- http://inl.info.ucl.ac.be
MultiPath TCP in the Linux Kernel --- http://mptcp.info.ucl.ac.be
UCLouvain
--

^ permalink raw reply

* Re: Doubts about listen backlog and tcp_max_syn_backlog
From: Eric Dumazet @ 2013-01-22 16:45 UTC (permalink / raw)
  To: Leandro Lucarella; +Cc: netdev, linux-kernel
In-Reply-To: <20130122161038.GG4608@sociomantic.com>

On Tue, 2013-01-22 at 17:10 +0100, Leandro Lucarella wrote:
> Hi, I'm having some problems with missing SYNs in a server with a high
> rate of incoming connections and, even when far from understanding the
> kernel,  I ended up looking at the kernel's source to try to understand
> better what's going on, because some stuff doesn't make a lot of sense
> to me.
> 
> The path I followed is this (line numbers for Linux 3.7):
> net/socket.c[3]
>     SYSCALL_DEFINE2(listen, int, fd, int, backlog)
>         backlog is truncated to sysctl_somaxconn and
>         sock->ops->listen(sock, backlog) is called, which I guess it
>         calls to inet_listen().
> 
> net/ipv4/af_inet.c[4]
>     int inet_listen(struct socket *sock, int backlog)
>         the backlog is assigned to sk->sk_max_ack_backlog and
>         inet_csk_listen_start(sk, backlog) is called (if the socket
>         wans't already in TCP_LISTEN state)
> 
> net/ipv4/inet_connection_sock.c[5]
>     int inet_csk_listen_start(struct sock *sk, const int nr_table_entries)
>         reqsk_queue_alloc(&icsk->icsk_accept_queue, nr_table_entries) is
>         called, which I guess it creates the actual queue
> 
> net/core/request_sock.c[6]
>     int reqsk_queue_alloc(struct request_sock_queue *queue,
>                           unsigned int nr_table_entries)
>         nr_table_entries is first adjusted to satisfy:
>         8 <= nr_table_entries <= sysctl_max_syn_backlog
>         and then incremented by one and rounded up to the next power of
>         2.
> 
> So here are a couple of questions:
> 
> 1. What's the relation between the socket backlog and the queue created
>    by reqsk_queue_alloc()? Because the backlog is only adjusted not to
>    be grater than sysctl_somaxconn, but the queue size can be quite
>    different.
> 2. The comment just above the definition of reqsk_queue_alloc() about
>    sysctl_max_syn_backlog says "Maximum number of SYN_RECV sockets in
>    queue per LISTEN socket.". But then nr_table_entries is not only
>    rounded up to the next power of 2, is incremented by one before that,
>    so a backlog of, for example, 128, would end up with 256 table
>    entries even if sysctl_max_syn_backlog is 128.
> 3. Why is there a nr_table_entries + 1 at all in there? Looking at the
>    commit that introduced this[1] I can't find any explanation and I've
>    read some big projects are using backlogs of 511 because of this[2].
>    (which BTW, ff the queue is really a hash table, looks like an awful
>    idea).
> 4. I found some places sk->sk_ack_backlog is checked against
>    sk->sk_max_ack_backlog to see if new requests should be dropped, but
>    I also saw checks like inet_csk_reqsk_queue_young(sk) > 1 or
>    inet_csk_reqsk_queue_is_full(sk), so I guess the queue is used too.
> 
> 
> Thanks a lot.
> 
> [1] http://git.kernel.org/?p=linux/kernel/git/stable/linux-stable.git;a=commitdiff;h=72a3effaf633bcae9034b7e176bdbd78d64a71db
> [2] http://blog.dubbelboer.com/2012/04/09/syn-cookies.html#a_reasonably_backlog_size
> [3] http://git.kernel.org/?p=linux/kernel/git/stable/linux-stable.git;a=blob;f=net/socket.c;h=2ca51c719ef984cdadef749008456cf7bd5e1ae4;hb=HEAD#l1544
> [4] http://git.kernel.org/?p=linux/kernel/git/stable/linux-stable.git;a=blob;f=net/ipv4/af_inet.c;h=24b384b7903ea7a59a11e7a4cbf06db996498924;hb=HEAD#l192
> [5] http://git.kernel.org/?p=linux/kernel/git/stable/linux-stable.git;a=blob;f=net/ipv4/inet_connection_sock.c;h=d0670f00d5243f95bec4536f60edf32fa2ded850;hb=HEAD#l729
> [6] http://git.kernel.org/?p=linux/kernel/git/stable/linux-stable.git;a=blob;f=net/core/request_sock.c;h=c31d9e8668c30346894adbf3be55eed4beeb1258;hb=HEAD#l23
> 


What particular problem do you have ?

A serious rewrite of LISTEN code is needed, because the current
implementation doesn't scale :

The SYNACK retransmits are done by a single timer wheel, holding the
socket lock for too long. So increasing the backlog to 2^16 or 2^17 is
not really an option.

Hash table are nice, but if we have to scan them, holding a single lock,
they are not so nice.

^ permalink raw reply

* Re: BUG in netxen_release_tx_buffers when TSO enabled on kernels >= 3.3 and <= 3.6
From: Eric Dumazet @ 2013-01-22 16:33 UTC (permalink / raw)
  To: christoph.paasch
  Cc: Ian Campbell, Sony Chacko, Rajesh Borundia, David Miller, netdev
In-Reply-To: <3172160.NCYvLW6RVJ@cpaasch-mac>

From: Eric Dumazet <edumazet@google.com>

On Tue, 2013-01-22 at 16:43 +0100, Christoph Paasch wrote:
> In netxen_map_tx_skb() I think we also have to set nf->dma to 0ULL (like the 
> diff below).
> 
> Otherwise, netxen_release_tx_buffer() may try to unmap something that has 
> already been unmapped.
> 
> I'm not sure - I don't feel very comfortable in driver-code...

It seems fine to me, here is the official combined patch, feel
free to add your 'Signed-off-by'

Thanks !

[PATCH] netxen: fix off by one bug in netxen_release_tx_buffer()

Christoph Paasch found netxen could trigger a BUG in its dismantle
phase, in netxen_release_tx_buffer(), using full size TSO packets.

cmd_buf->frag_count includes the skb->data part, so the loop must
start at index 1 instead of 0, or else we can make an out
of bound access to cmd_buff->frag_array[MAX_SKB_FRAGS + 2]

Christoph provided the fixes in netxen_map_tx_skb() function.
In case of a dma mapping error, its better to clear the dma fields
so that we don't try to unmap them again in netxen_release_tx_buffer()

Reported-by: Christoph Paasch <christoph.paasch@uclouvain.be>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Tested-by: Christoph Paasch <christoph.paasch@uclouvain.be>
Cc: Sony Chacko <sony.chacko@qlogic.com>
Cc: Rajesh Borundia <rajesh.borundia@qlogic.com>
---
 drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c |    2 +-
 drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c |    2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c b/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c
index bc165f4..695667d 100644
--- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c
+++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c
@@ -144,7 +144,7 @@ void netxen_release_tx_buffers(struct netxen_adapter *adapter)
 					 buffrag->length, PCI_DMA_TODEVICE);
 			buffrag->dma = 0ULL;
 		}
-		for (j = 0; j < cmd_buf->frag_count; j++) {
+		for (j = 1; j < cmd_buf->frag_count; j++) {
 			buffrag++;
 			if (buffrag->dma) {
 				pci_unmap_page(adapter->pdev, buffrag->dma,
diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
index 6098fd4a..69e321a 100644
--- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
+++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
@@ -1963,10 +1963,12 @@ unwind:
 	while (--i >= 0) {
 		nf = &pbuf->frag_array[i+1];
 		pci_unmap_page(pdev, nf->dma, nf->length, PCI_DMA_TODEVICE);
+		nf->dma = 0ULL;
 	}
 
 	nf = &pbuf->frag_array[0];
 	pci_unmap_single(pdev, nf->dma, skb_headlen(skb), PCI_DMA_TODEVICE);
+	nf->dma = 0ULL;
 
 out_err:
 	return -ENOMEM;

^ permalink raw reply related

* [PATCH net-next 4/4] netfilter: Use IS_ERR_OR_NULL().
From: YOSHIFUJI Hideaki @ 2013-01-22 16:33 UTC (permalink / raw)
  To: davem, netdev; +Cc: yoshfuji

Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
 net/ipv4/netfilter/arp_tables.c |   10 +++++-----
 net/ipv4/netfilter/ip_tables.c  |   10 +++++-----
 net/ipv6/netfilter/ip6_tables.c |   10 +++++-----
 3 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c
index 3ea4127..7dc6a97 100644
--- a/net/ipv4/netfilter/arp_tables.c
+++ b/net/ipv4/netfilter/arp_tables.c
@@ -901,7 +901,7 @@ static int get_info(struct net *net, void __user *user,
 #endif
 	t = try_then_request_module(xt_find_table_lock(net, NFPROTO_ARP, name),
 				    "arptable_%s", name);
-	if (t && !IS_ERR(t)) {
+	if (!IS_ERR_OR_NULL(t)) {
 		struct arpt_getinfo info;
 		const struct xt_table_info *private = t->private;
 #ifdef CONFIG_COMPAT
@@ -958,7 +958,7 @@ static int get_entries(struct net *net, struct arpt_get_entries __user *uptr,
 	}
 
 	t = xt_find_table_lock(net, NFPROTO_ARP, get.name);
-	if (t && !IS_ERR(t)) {
+	if (!IS_ERR_OR_NULL(t)) {
 		const struct xt_table_info *private = t->private;
 
 		duprintf("t->private->number = %u\n",
@@ -1001,7 +1001,7 @@ static int __do_replace(struct net *net, const char *name,
 
 	t = try_then_request_module(xt_find_table_lock(net, NFPROTO_ARP, name),
 				    "arptable_%s", name);
-	if (!t || IS_ERR(t)) {
+	if (IS_ERR_OR_NULL(t)) {
 		ret = t ? PTR_ERR(t) : -ENOENT;
 		goto free_newinfo_counters_untrans;
 	}
@@ -1158,7 +1158,7 @@ static int do_add_counters(struct net *net, const void __user *user,
 	}
 
 	t = xt_find_table_lock(net, NFPROTO_ARP, name);
-	if (!t || IS_ERR(t)) {
+	if (IS_ERR_OR_NULL(t)) {
 		ret = t ? PTR_ERR(t) : -ENOENT;
 		goto free;
 	}
@@ -1646,7 +1646,7 @@ static int compat_get_entries(struct net *net,
 
 	xt_compat_lock(NFPROTO_ARP);
 	t = xt_find_table_lock(net, NFPROTO_ARP, get.name);
-	if (t && !IS_ERR(t)) {
+	if (!IS_ERR_OR_NULL(t)) {
 		const struct xt_table_info *private = t->private;
 		struct xt_table_info info;
 
diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index 17c5e06..3efcf87 100644
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -1090,7 +1090,7 @@ static int get_info(struct net *net, void __user *user,
 #endif
 	t = try_then_request_module(xt_find_table_lock(net, AF_INET, name),
 				    "iptable_%s", name);
-	if (t && !IS_ERR(t)) {
+	if (!IS_ERR_OR_NULL(t)) {
 		struct ipt_getinfo info;
 		const struct xt_table_info *private = t->private;
 #ifdef CONFIG_COMPAT
@@ -1149,7 +1149,7 @@ get_entries(struct net *net, struct ipt_get_entries __user *uptr,
 	}
 
 	t = xt_find_table_lock(net, AF_INET, get.name);
-	if (t && !IS_ERR(t)) {
+	if (!IS_ERR_OR_NULL(t)) {
 		const struct xt_table_info *private = t->private;
 		duprintf("t->private->number = %u\n", private->number);
 		if (get.size == private->size)
@@ -1189,7 +1189,7 @@ __do_replace(struct net *net, const char *name, unsigned int valid_hooks,
 
 	t = try_then_request_module(xt_find_table_lock(net, AF_INET, name),
 				    "iptable_%s", name);
-	if (!t || IS_ERR(t)) {
+	if (IS_ERR_OR_NULL(t)) {
 		ret = t ? PTR_ERR(t) : -ENOENT;
 		goto free_newinfo_counters_untrans;
 	}
@@ -1347,7 +1347,7 @@ do_add_counters(struct net *net, const void __user *user,
 	}
 
 	t = xt_find_table_lock(net, AF_INET, name);
-	if (!t || IS_ERR(t)) {
+	if (IS_ERR_OR_NULL(t)) {
 		ret = t ? PTR_ERR(t) : -ENOENT;
 		goto free;
 	}
@@ -1931,7 +1931,7 @@ compat_get_entries(struct net *net, struct compat_ipt_get_entries __user *uptr,
 
 	xt_compat_lock(AF_INET);
 	t = xt_find_table_lock(net, AF_INET, get.name);
-	if (t && !IS_ERR(t)) {
+	if (!IS_ERR_OR_NULL(t)) {
 		const struct xt_table_info *private = t->private;
 		struct xt_table_info info;
 		duprintf("t->private->number = %u\n", private->number);
diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
index 125a90d..341b54a 100644
--- a/net/ipv6/netfilter/ip6_tables.c
+++ b/net/ipv6/netfilter/ip6_tables.c
@@ -1098,7 +1098,7 @@ static int get_info(struct net *net, void __user *user,
 #endif
 	t = try_then_request_module(xt_find_table_lock(net, AF_INET6, name),
 				    "ip6table_%s", name);
-	if (t && !IS_ERR(t)) {
+	if (!IS_ERR_OR_NULL(t)) {
 		struct ip6t_getinfo info;
 		const struct xt_table_info *private = t->private;
 #ifdef CONFIG_COMPAT
@@ -1157,7 +1157,7 @@ get_entries(struct net *net, struct ip6t_get_entries __user *uptr,
 	}
 
 	t = xt_find_table_lock(net, AF_INET6, get.name);
-	if (t && !IS_ERR(t)) {
+	if (!IS_ERR_OR_NULL(t)) {
 		struct xt_table_info *private = t->private;
 		duprintf("t->private->number = %u\n", private->number);
 		if (get.size == private->size)
@@ -1197,7 +1197,7 @@ __do_replace(struct net *net, const char *name, unsigned int valid_hooks,
 
 	t = try_then_request_module(xt_find_table_lock(net, AF_INET6, name),
 				    "ip6table_%s", name);
-	if (!t || IS_ERR(t)) {
+	if (IS_ERR_OR_NULL(t)) {
 		ret = t ? PTR_ERR(t) : -ENOENT;
 		goto free_newinfo_counters_untrans;
 	}
@@ -1355,7 +1355,7 @@ do_add_counters(struct net *net, const void __user *user, unsigned int len,
 	}
 
 	t = xt_find_table_lock(net, AF_INET6, name);
-	if (!t || IS_ERR(t)) {
+	if (IS_ERR_OR_NULL(t)) {
 		ret = t ? PTR_ERR(t) : -ENOENT;
 		goto free;
 	}
@@ -1939,7 +1939,7 @@ compat_get_entries(struct net *net, struct compat_ip6t_get_entries __user *uptr,
 
 	xt_compat_lock(AF_INET6);
 	t = xt_find_table_lock(net, AF_INET6, get.name);
-	if (t && !IS_ERR(t)) {
+	if (!IS_ERR_OR_NULL(t)) {
 		const struct xt_table_info *private = t->private;
 		struct xt_table_info info;
 		duprintf("t->private->number = %u\n", private->number);
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH net-next 3/4] ipv6: Use IS_ERR_OR_NULL().
From: YOSHIFUJI Hideaki @ 2013-01-22 16:32 UTC (permalink / raw)
  To: davem, netdev; +Cc: yoshfuji

Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
 net/ipv6/addrconf.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 420e563..80d5980 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -1051,7 +1051,7 @@ retry:
 		ipv6_add_addr(idev, &addr, tmp_plen,
 			      ipv6_addr_type(&addr)&IPV6_ADDR_SCOPE_MASK,
 			      addr_flags) : NULL;
-	if (!ift || IS_ERR(ift)) {
+	if (IS_ERR_OR_NULL(ift)) {
 		in6_ifa_put(ifp);
 		in6_dev_put(idev);
 		pr_info("%s: retry temporary address regeneration\n", __func__);
@@ -2079,7 +2079,7 @@ ok:
 						    addr_type&IPV6_ADDR_SCOPE_MASK,
 						    addr_flags);
 
-			if (!ifp || IS_ERR(ifp)) {
+			if (IS_ERR_OR_NULL(ifp)) {
 				in6_dev_put(in6_dev);
 				return;
 			}
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH net-next 2/4] ipv4: Use IS_ERR_OR_NULL().
From: YOSHIFUJI Hideaki @ 2013-01-22 16:32 UTC (permalink / raw)
  To: davem, netdev; +Cc: yoshfuji

Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
 net/ipv4/af_inet.c |    2 +-
 net/ipv4/tcp.c     |    2 +-
 net/ipv4/udp.c     |    2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 4fdf967..4b70539 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -1333,7 +1333,7 @@ static struct sk_buff *inet_gso_segment(struct sk_buff *skb,
 		segs = ops->callbacks.gso_segment(skb, features);
 	rcu_read_unlock();
 
-	if (!segs || IS_ERR(segs))
+	if (IS_ERR_OR_NULL(segs))
 		goto out;
 
 	skb = segs;
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 2aa69c8..5227194 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -3243,7 +3243,7 @@ __tcp_alloc_md5sig_pool(struct sock *sk)
 		struct crypto_hash *hash;
 
 		hash = crypto_alloc_hash("md5", 0, CRYPTO_ALG_ASYNC);
-		if (!hash || IS_ERR(hash))
+		if (IS_ERR_OR_NULL(hash))
 			goto out_free;
 
 		per_cpu_ptr(pool, cpu)->md5_desc.tfm = hash;
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 79c8dbe..cf6158f 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -971,7 +971,7 @@ back_from_confirm:
 				  sizeof(struct udphdr), &ipc, &rt,
 				  msg->msg_flags);
 		err = PTR_ERR(skb);
-		if (skb && !IS_ERR(skb))
+		if (!IS_ERR_OR_NULL(skb))
 			err = udp_send_skb(skb, fl4);
 		goto out;
 	}
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH net-next 1/4] net: Use IS_ERR_OR_NULL().
From: YOSHIFUJI Hideaki @ 2013-01-22 16:32 UTC (permalink / raw)
  To: davem, netdev; +Cc: yoshfuji

Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
 net/core/flow.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/core/flow.c b/net/core/flow.c
index b0901ee..43f7495 100644
--- a/net/core/flow.c
+++ b/net/core/flow.c
@@ -286,7 +286,7 @@ nocache:
 		else
 			fle->genid--;
 	} else {
-		if (flo && !IS_ERR(flo))
+		if (!IS_ERR_OR_NULL(flo))
 			flo->ops->delete(flo);
 	}
 ret_object:
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH net-next 0/4] Use IS_ERR_OR_NULL().
From: YOSHIFUJI Hideaki @ 2013-01-22 16:32 UTC (permalink / raw)
  To: davem, netdev; +Cc: yoshfuji

YOSHIFUJI Hideaki (4):
  net: Use IS_ERR_OR_NULL().
  ipv4: Use IS_ERR_OR_NULL().
  ipv6: Use IS_ERR_OR_NULL().
  netfilter: Use IS_ERR_OR_NULL().

 net/core/flow.c                 |    2 +-
 net/ipv4/af_inet.c              |    2 +-
 net/ipv4/netfilter/arp_tables.c |   10 +++++-----
 net/ipv4/netfilter/ip_tables.c  |   10 +++++-----
 net/ipv4/tcp.c                  |    2 +-
 net/ipv4/udp.c                  |    2 +-
 net/ipv6/addrconf.c             |    4 ++--
 net/ipv6/netfilter/ip6_tables.c |   10 +++++-----
 8 files changed, 21 insertions(+), 21 deletions(-)

-- 
1.7.9.5

^ permalink raw reply

* Re: [PATCH net-next V6 02/14] bridge: Add vlan filtering infrastructure
From: Vlad Yasevich @ 2013-01-22 16:27 UTC (permalink / raw)
  To: Shmulik Ladkani
  Cc: bridge, Stephen Hemminger, Michał Mirosław, netdev,
	davem, shemminger, mst
In-Reply-To: <20130122175524.033c6f7f.shmulik.ladkani@gmail.com>

On 01/22/2013 10:55 AM, Shmulik Ladkani wrote:
> Thanks Vlad,
>
> On Tue, 22 Jan 2013 09:31:43 -0500 Vlad Yasevich <vyasevic@redhat.com> wrote:
>>> I guess this might simplify the data structures involved, avoiding the
>>> refcounts, etc...
>>>
>>> The penaties are:
>>>    - memory
>>>    - aesthetics (?)
>>>    - inefficient if query is "give me the entire list of VLANs port P is
>>>      member of". But do we have such a query in bridge's code?
>>
>> Yes.  When a mac address is added to a port without an explicit vlan tag
>> we try to add it for every vlan available on the port.
>
> I see.
> Can't this be bypassed by adding a _single_ FDB entry whose VID value
> denotes "member of ANY vlan" (value outside the valid 0-4095 range)?
>
>> Also, in the API, the user may request vlans configured on a port.
>
> Personally I'd pay the penalty implementing this specific user request
> in an inefficeint way, to acheive overall simplicity in core bridge
> code.
> But that's just my humble opinion, maybe others might spot drawbacks
> taking this approach.
>
> BTW, went through the ML, couldn't find the reason why dropped the
> per-port vlan bitmap and replaced with a vlan list (after your RFC v2
> patches). Care to explain what was your motivation?

I wanted to reduce the memory footprint and make it a bit more 
extensible so if priority was ever added, it would be very simple to do.
I also had to play some ugly memory barrier games to make it less racy.
I thought that the list/hash code was cleaner.

-vlad

>
> Regards,
> Shmulik
>

^ permalink raw reply

* Doubts about listen backlog and tcp_max_syn_backlog
From: Leandro Lucarella @ 2013-01-22 16:10 UTC (permalink / raw)
  To: netdev, linux-kernel

Hi, I'm having some problems with missing SYNs in a server with a high
rate of incoming connections and, even when far from understanding the
kernel,  I ended up looking at the kernel's source to try to understand
better what's going on, because some stuff doesn't make a lot of sense
to me.

The path I followed is this (line numbers for Linux 3.7):
net/socket.c[3]
    SYSCALL_DEFINE2(listen, int, fd, int, backlog)
        backlog is truncated to sysctl_somaxconn and
        sock->ops->listen(sock, backlog) is called, which I guess it
        calls to inet_listen().

net/ipv4/af_inet.c[4]
    int inet_listen(struct socket *sock, int backlog)
        the backlog is assigned to sk->sk_max_ack_backlog and
        inet_csk_listen_start(sk, backlog) is called (if the socket
        wans't already in TCP_LISTEN state)

net/ipv4/inet_connection_sock.c[5]
    int inet_csk_listen_start(struct sock *sk, const int nr_table_entries)
        reqsk_queue_alloc(&icsk->icsk_accept_queue, nr_table_entries) is
        called, which I guess it creates the actual queue

net/core/request_sock.c[6]
    int reqsk_queue_alloc(struct request_sock_queue *queue,
                          unsigned int nr_table_entries)
        nr_table_entries is first adjusted to satisfy:
        8 <= nr_table_entries <= sysctl_max_syn_backlog
        and then incremented by one and rounded up to the next power of
        2.

So here are a couple of questions:

1. What's the relation between the socket backlog and the queue created
   by reqsk_queue_alloc()? Because the backlog is only adjusted not to
   be grater than sysctl_somaxconn, but the queue size can be quite
   different.
2. The comment just above the definition of reqsk_queue_alloc() about
   sysctl_max_syn_backlog says "Maximum number of SYN_RECV sockets in
   queue per LISTEN socket.". But then nr_table_entries is not only
   rounded up to the next power of 2, is incremented by one before that,
   so a backlog of, for example, 128, would end up with 256 table
   entries even if sysctl_max_syn_backlog is 128.
3. Why is there a nr_table_entries + 1 at all in there? Looking at the
   commit that introduced this[1] I can't find any explanation and I've
   read some big projects are using backlogs of 511 because of this[2].
   (which BTW, ff the queue is really a hash table, looks like an awful
   idea).
4. I found some places sk->sk_ack_backlog is checked against
   sk->sk_max_ack_backlog to see if new requests should be dropped, but
   I also saw checks like inet_csk_reqsk_queue_young(sk) > 1 or
   inet_csk_reqsk_queue_is_full(sk), so I guess the queue is used too.


Thanks a lot.

[1] http://git.kernel.org/?p=linux/kernel/git/stable/linux-stable.git;a=commitdiff;h=72a3effaf633bcae9034b7e176bdbd78d64a71db
[2] http://blog.dubbelboer.com/2012/04/09/syn-cookies.html#a_reasonably_backlog_size
[3] http://git.kernel.org/?p=linux/kernel/git/stable/linux-stable.git;a=blob;f=net/socket.c;h=2ca51c719ef984cdadef749008456cf7bd5e1ae4;hb=HEAD#l1544
[4] http://git.kernel.org/?p=linux/kernel/git/stable/linux-stable.git;a=blob;f=net/ipv4/af_inet.c;h=24b384b7903ea7a59a11e7a4cbf06db996498924;hb=HEAD#l192
[5] http://git.kernel.org/?p=linux/kernel/git/stable/linux-stable.git;a=blob;f=net/ipv4/inet_connection_sock.c;h=d0670f00d5243f95bec4536f60edf32fa2ded850;hb=HEAD#l729
[6] http://git.kernel.org/?p=linux/kernel/git/stable/linux-stable.git;a=blob;f=net/core/request_sock.c;h=c31d9e8668c30346894adbf3be55eed4beeb1258;hb=HEAD#l23

-- 
Leandro Lucarella
sociomantic labs GmbH
http://www.sociomantic.com

^ permalink raw reply

* Re: [PATCH net-next V6 02/14] bridge: Add vlan filtering infrastructure
From: Shmulik Ladkani @ 2013-01-22 15:55 UTC (permalink / raw)
  To: vyasevic
  Cc: bridge, Stephen Hemminger, Michał Mirosław, netdev,
	davem, shemminger, mst
In-Reply-To: <50FEA2CF.4060406@redhat.com>

Thanks Vlad,

On Tue, 22 Jan 2013 09:31:43 -0500 Vlad Yasevich <vyasevic@redhat.com> wrote:
> > I guess this might simplify the data structures involved, avoiding the
> > refcounts, etc...
> >
> > The penaties are:
> >   - memory
> >   - aesthetics (?)
> >   - inefficient if query is "give me the entire list of VLANs port P is
> >     member of". But do we have such a query in bridge's code?
> 
> Yes.  When a mac address is added to a port without an explicit vlan tag 
> we try to add it for every vlan available on the port.

I see.
Can't this be bypassed by adding a _single_ FDB entry whose VID value
denotes "member of ANY vlan" (value outside the valid 0-4095 range)?

> Also, in the API, the user may request vlans configured on a port.

Personally I'd pay the penalty implementing this specific user request
in an inefficeint way, to acheive overall simplicity in core bridge
code.
But that's just my humble opinion, maybe others might spot drawbacks
taking this approach.

BTW, went through the ML, couldn't find the reason why dropped the
per-port vlan bitmap and replaced with a vlan list (after your RFC v2
patches). Care to explain what was your motivation?

Regards,
Shmulik

^ permalink raw reply

* RE: [PATCH net 2/3] net: cdc_mbim: send ZLP after max sized NTBs
From: Alexey ORISHKO @ 2013-01-22 15:51 UTC (permalink / raw)
  To: Bjørn Mork
  Cc: netdev@vger.kernel.org, linux-usb@vger.kernel.org, Greg Suarez,
	Oliver Neukum, Alexey Orishko
In-Reply-To: <87zk01a6gj.fsf@nemi.mork.no>

H Bjørn,

> -----Original Message-----
> From: Bjørn Mork [mailto:bjorn@mork.no]
> Sent: Tuesday, January 22, 2013 10:54 AM
> 
> > If you add ZLP for NTBs of dwNtbOutMaxSize, you are heavily affecting
> > CPU load, increasing interrupt load by factor of 2 in high load
> > traffic scenario and possibly decreasing throughput for all other
> > devices which behaves correctly.
>  
> The current cdc_ncm/cdc_mbim drivers will pad a NTB to the full
> dwNtbOutMaxSize whenever it reaches at least 512 bytes.  The reason is
> that this allows more efficient device DMA operation.  This is
> something we do to adapt to device hardware restrictions even though
> there is no such recommendations in the NCM/MBIM specs.  

There are a lot of things which were discussed during development of
specifications, but not ended up in the final version of the spec due to
various reasons. Companies attending USB-IF F2F meetings can benefit
from discussions between member companies and get access to additional
information not visible outside of USB-IF. 

> The penalty on
> the host and bus should be obvious: Even with a quite small
> dwNtbOutMaxSize of 4096, we end up sending 8 x 512-byte data packets
> instead of the 2 we could have managed with.

It was intentional and it's developer choice (see also the last comment).
It's a straightforward approach, but this limit could be a dynamic
value based on statistic and current load.

>  
> Now you claim that sending 9 packets, where the last one is a zero
> length packet, increase the interrupt load by factor of 2?  How is
> that?

You set up DMA job to receive full NTB and get a single interrupt
when job is done. It means while DMA is collecting data, CPU can do
something else (send data to NW stack).

In other protocols you need to indicate the end of data, but in
NCM/MBIM we know for sure that host is not allowed to send more
than dwNtbOutMaxSize bytes, so ZLP is not needed.

If host decides to send ZLP after full NTB, CPU must handle
additional INT per every full NTB instead of doing useful work.
For FTP transfer with constantly full NTBs you get twice amount of
Interrupts.

> Why is the device DMA restrictions not a fault, while the device ZLP
> requirement is?  Both seem like reasonable device hardware/firmware
> implementation imposed restrictions to me.  Something we'll just have
> to accept.

Both specifications (NCM/MBIM) were written from the point of host (most
likely PC with 2/4 core CPU) being more powerful than device and with
requirement for host to honor device limitations.

Regards,
Alexey

^ permalink raw reply

* Re: [PATCH net-next 01/15] drivers/net: delete 486 Apricot support
From: Paul Gortmaker @ 2013-01-22 15:51 UTC (permalink / raw)
  To: David Miller; +Cc: netdev
In-Reply-To: <1358819182-28032-2-git-send-email-paul.gortmaker@windriver.com>

[[PATCH net-next 01/15] drivers/net: delete 486 Apricot support] On 21/01/2013 (Mon 20:46) Paul Gortmaker wrote:

[...]

>  
> -#elif defined(ENABLE_APRICOT)
> -
> -	{
> -		u32 scp = virt_to_bus(&lp->scp);
> -
> -		/* change the scp address */
> -		outw(0, ioaddr);
> -		outw(0, ioaddr);
> -		outb(4, ioaddr + 0xf);
> -		outw(scp | 2, ioaddr);
> -		outw(scp >> 16, ioaddr);
> -	}
> -#endif
> -

I deleted one too many lines here.  That last #endif needs
to stay as the parent #if still remains.  Fengguang's 0-day
testing found it in an m68k allyesconfig build.

I've folded this in, build tested for m68k locally, and done
a forced update on my legacy-isa-delete branch.

Paul.
--

^ permalink raw reply

* Re: BUG in netxen_release_tx_buffers when TSO enabled on kernels >= 3.3 and <= 3.6
From: Christoph Paasch @ 2013-01-22 15:43 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Ian Campbell, Sony Chacko, Rajesh Borundia, David Miller, netdev
In-Reply-To: <2166577.r0iRQtU7qE@cpaasch-mac>

In netxen_map_tx_skb() I think we also have to set nf->dma to 0ULL (like the 
diff below).

Otherwise, netxen_release_tx_buffer() may try to unmap something that has 
already been unmapped.

I'm not sure - I don't feel very comfortable in driver-code...


diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c 
b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
index 342b3a7..a1516a6 100644
--- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
+++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
@@ -1959,10 +1959,12 @@ unwind:
        while (--i >= 0) {
                nf = &pbuf->frag_array[i+1];
                pci_unmap_page(pdev, nf->dma, nf->length, PCI_DMA_TODEVICE);
+               nf->dma = 0ULL;
        }
 
        nf = &pbuf->frag_array[0];
        pci_unmap_single(pdev, nf->dma, skb_headlen(skb), PCI_DMA_TODEVICE);
+       nf->dma = 0ULL;
 
 out_err:
        return -ENOMEM;



On Tuesday 22 January 2013 16:03:59 Christoph Paasch wrote:
> On Tuesday 22 January 2013 05:56:06 Eric Dumazet wrote:
> > I guess netxen driver has a bug.
> > 
> > Please try the following patch :
> > 
> > diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c
> > b/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c index
> > bc165f4..695667d 100644
> > --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c
> > +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c
> > @@ -144,7 +144,7 @@ void netxen_release_tx_buffers(struct netxen_adapter
> > *adapter) buffrag->length, PCI_DMA_TODEVICE); buffrag->dma = 0ULL;
> > 
> >                 }
> > 
> > -               for (j = 0; j < cmd_buf->frag_count; j++) {
> > +               for (j = 1; j < cmd_buf->frag_count; j++) {
> > 
> >                         buffrag++;
> >                         if (buffrag->dma) {
> >                         
> >                                 pci_unmap_page(adapter->pdev,
> >                                 buffrag->dma,
> 
> Perfect, I tested it, and this fixes the bug.
> 
> I should have found it on my own, I have been starring for too long at this
> function... :)
> 
> Feel free to add
> Tested-by: Christoph Paasch <christoph.paasch@uclouvain.be>
> 
> 
> Thanks, Eric!
> 
> 
> Christoph
-- 
IP Networking Lab --- http://inl.info.ucl.ac.be
MultiPath TCP in the Linux Kernel --- http://mptcp.info.ucl.ac.be
UCLouvain
--

^ permalink raw reply related

* Re: [PATCH] net: fec: Add support for multiple phys on mdiobus
From: Wolfgang Grandegger @ 2013-01-22 15:26 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: Sascha Hauer, netdev, linux-arm-kernel, shawn.guo, davem
In-Reply-To: <50FEAE12.7090307@openwrt.org>

On 01/22/2013 04:19 PM, Florian Fainelli wrote:
> On 01/22/2013 04:03 PM, Wolfgang Grandegger wrote:
>> On 01/22/2013 03:47 PM, Florian Fainelli wrote:
>>> On 01/22/2013 08:22 AM, Wolfgang Grandegger wrote:
>>>>> Well this could be done when the fixed phy driver could be registered
>>>>> with the devicetree, maybe like this:
>>>>>
>>>>>      fixed-phy: mdiophy {
>>>>>          compatible = "mdio-fixed-phy";
>>>>>          link = "100FD";
>>>>>      };
>>>> I find that confusing. There is *no* phy but just a fixed link to the
>>>> switch...
>>>>
>>>>> The good thing about this would be that every ethernet driver could
>>>>> just
>>>>> use such a fixed phy, any external mdio phy (like on Marvell
>>>>> Armada) or
>>>>> just a phy connected to the internal mdio interface provided by the
>>>>> ethernet
>>>>> core.
>>>> What is wrong with the existing "fixed-link" property of the *ethernet*
>>>> node. The fixed-link handling should/could be done in the phy layer,
>>>> and
>>>> not in the driver as it currently is implemented. Maybe that's the
>>>> reason why the current code is regarded as hack!
>>> As far as I have used it with the CPMAC driver, the fixed PHY is a
>>> specific PHY device and there is no specific handling to be done by the
>>> Ethernet MAC driver but eventually changing its MII bus id so that it is
>>> named "fixed-0" to allow the fixed PHY driver to bind. Put differently,
>> There is special handling for the fixed link, e.g. here:
>>
>> http://lxr.linux.no/#linux+v3.7.4/drivers/net/ethernet/freescale/gianfar.c#L1462
>>
>>
>> This could be hidden in the PHY layer allowing all ethernet drivers
>> using the "fixed-link" property.
> 
> Ok, so in the end you could have potentially an ethernet PHY node
> containing the following properties:
> - fixed-link
> - fixed-speed
> - fixed-duplex
> 
> and treat it as a fixed-phy. If so, this would be pretty handy.

The "fixed-link" property already allows to specify speed, duplex, etc.:

http://lxr.linux.no/#linux+v3.7.4/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt#L48

Wolfgang.

^ permalink raw reply

* Re: 69b08f62 breaks ethernet on atl1c based Eee PC 1011px
From: Michael Büsch @ 2013-01-22 15:25 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: linux-netdev, Alexander Duyck, Benjamin LaHaise, David S. Miller,
	Jay Cliburn, Chris Snook
In-Reply-To: <CANn89iK5iEL5HnGxtRR2Gk_EEcV5wUmiAB9XgNwn9c-+c2M5gw@mail.gmail.com>

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

On Tue, 22 Jan 2013 07:16:49 -0800
Eric Dumazet <edumazet@google.com> wrote:

> Its probably a VM issue fixed by Mel Gorman in
> commit 8fb74b9fb2b182d54beee592350d9ea1f325917a
> 
> mm: compaction: partially revert capture of suitable high-order page

That was already applied in 3.7.3 as e5e65a5233c3f368c442413cebe7de2a4580b564,
so it doesn't fix the issue.

-- 
Greetings, Michael.

PGP: 908D8B0E

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ 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