Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next v2.1 3/3] bonding: update bonding.txt for primary description
From: Ding Tianhong @ 2014-01-14 10:54 UTC (permalink / raw)
  To: Jay Vosburgh, Veaceslav Falico, David S. Miller, Netdev

Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
 Documentation/networking/bonding.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/networking/bonding.txt b/Documentation/networking/bonding.txt
index a4d925e..5cdb229 100644
--- a/Documentation/networking/bonding.txt
+++ b/Documentation/networking/bonding.txt
@@ -657,7 +657,8 @@ primary
 	one slave is preferred over another, e.g., when one slave has
 	higher throughput than another.
 
-	The primary option is only valid for active-backup mode.
+	The primary option is only valid for active-backup(1),
+	balance-tlb (5) and balance-alb (6) mode.
 
 primary_reselect
 
-- 
1.8.0

^ permalink raw reply related

* [PATCH net-next v2.1 1/3] bonding: update the primary slave when changing slave's name
From: Ding Tianhong @ 2014-01-14 10:54 UTC (permalink / raw)
  To: Jay Vosburgh, Veaceslav Falico, David S. Miller, Netdev

If the slave's name changed, and the bond params primary is exist,
the bond should deal with the situation in two ways:

1) If the slave was the primary slave yet, clean the primary slave
   and reselect active slave.
2) If the slave's new name is as same as bond primary, set the slave
   as primary slave and reselect active slave.

Thanks for Veaceslav's suggestion.

Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
 drivers/net/bonding/bond_main.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index e06c445..64e25d5 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2860,9 +2860,29 @@ static int bond_slave_netdev_event(unsigned long event,
 		 */
 		break;
 	case NETDEV_CHANGENAME:
-		/*
-		 * TODO: handle changing the primary's name
+		/* Handle changing the slave's name:
+		 * 1) If the slave was primary slave,
+		 * clean the primary slave and reselect
+		 * active slave.
+		 * 2) If the slave's new name is same as
+		 * bond primary, set the slave as primary
+		 * slave and reselect active slave.
 		 */
+		if (slave == bond->primary_slave ||
+		    !strcmp(bond->params.primary, slave_dev->name)) {
+			if (bond->primary_slave) {
+				pr_info("%s: Setting primary slave to None.\n",
+					bond->dev->name);
+				bond->primary_slave = NULL;
+			} else {
+				pr_info("%s: Setting %s as primary slave.\n",
+					bond->dev->name, slave_dev->name);
+				bond->primary_slave = slave;
+			}
+			write_lock_bh(&bond->curr_slave_lock);
+			bond_select_active_slave(bond);
+			write_unlock_bh(&bond->curr_slave_lock);
+		}
 		break;
 	case NETDEV_FEAT_CHANGE:
 		bond_compute_features(bond);
-- 
1.8.0

^ permalink raw reply related

* Re: [PATCH RESEND net-next v2 1/3] bonding: update the primary slave when changing slave's name
From: Veaceslav Falico @ 2014-01-14 10:51 UTC (permalink / raw)
  To: Ding Tianhong; +Cc: Jay Vosburgh, David S. Miller, Netdev
In-Reply-To: <52D4FE85.6050101@huawei.com>

On Tue, Jan 14, 2014 at 05:08:21PM +0800, Ding Tianhong wrote:
>If the slave's name changed, and the bond params primary is exist,
>the bond should deal with the situation in two ways:
>
>1) If the slave was the primary slave yet, clean the primary slave
>   and reselect active slave.
>2) If the slave's new name is as same as bond primary, set the slave
>   as primary slave and reselect active slave.
>
>Thanks for Veaceslav's suggestion.
>
>Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
>---
> drivers/net/bonding/bond_main.c | 24 ++++++++++++++++++++++--
> 1 file changed, 22 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index e06c445..64e25d5 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -2860,9 +2860,29 @@ static int bond_slave_netdev_event(unsigned long event,
> 		 */
> 		break;
> 	case NETDEV_CHANGENAME:
>-		/*
>-		 * TODO: handle changing the primary's name
>+		/* Handle changing the slave's name:
>+		 * 1) If the slave was primary slave,
>+		 * clean the primary slave and reselect
>+		 * active slave.
>+		 * 2) If the slave's new name is same as
>+		 * bond primary, set the slave as primary
>+		 * slave and reselect active slave.
> 		 */
>+		if (slave == bond->primary_slave ||
>+		    !strcmp(bond->params.primary, slave_dev->name)) {

And if we're in a mode that doesn't use primary, but have the
params.primary set? Then we'll issue a bond_select_active_slave() in, say,
802.3ad mode.

In the past 24h I've nacked about 5 of your patchsets, with you keeping
'quickfixing' them, without getting your time to understand the issues, and
re-sending them for review.

I'm not willing to waste my time that uselessly, reviewing patchsets that
you randomly generate in the hope of getting it right. And given your
'good' history - with patchsets that cause regressions and bugs, with
reverts because of that, with those horrible, meaningless RCU transition
that is just plainly wrong and *really* hard to fix - I'm going to react as
Greg KH said in one of his presentations - NAK your patches and make them
by myself. It will take *a lot* lesser time from my side, and will
eventually make the code better.

Thanks for the report, I'll send a patch that fixes it soon.

Nacked-by: Veaceslav Falico <vfalico@redhat.com>

>+			if (bond->primary_slave) {
>+				pr_info("%s: Setting primary slave to None.\n",
>+					bond->dev->name);
>+				bond->primary_slave = NULL;
>+			} else {
>+				pr_info("%s: Setting %s as primary slave.\n",
>+					bond->dev->name, slave_dev->name);
>+				bond->primary_slave = slave;
>+			}
>+			write_lock_bh(&bond->curr_slave_lock);
>+			bond_select_active_slave(bond);
>+			write_unlock_bh(&bond->curr_slave_lock);
>+		}
> 		break;
> 	case NETDEV_FEAT_CHANGE:
> 		bond_compute_features(bond);
>-- 
>1.8.0
>
>

^ permalink raw reply

* Re: [PATCH] netfilter: nf_conntrack: fix RCU race in nf_conntrack_find_get (v3)
From: Andrew Vagin @ 2014-01-14 10:51 UTC (permalink / raw)
  To: Eric Dumazet, Florian Westphal
  Cc: Andrey Vagin, netfilter-devel, netfilter, coreteam, netdev,
	linux-kernel, vvs, Pablo Neira Ayuso, Patrick McHardy,
	Jozsef Kadlecsik, David S. Miller, Cyrill Gorcunov
In-Reply-To: <1389558074.31367.187.camel@edumazet-glaptop2.roam.corp.google.com>

On Sun, Jan 12, 2014 at 12:21:14PM -0800, Eric Dumazet wrote:
> On Sun, 2014-01-12 at 21:50 +0400, Andrey Vagin wrote:
> > Lets look at destroy_conntrack:
> > 
> > hlist_nulls_del_rcu(&ct->tuplehash[IP_CT_DIR_ORIGINAL].hnnode);
> > ...
> > nf_conntrack_free(ct)
> > 	kmem_cache_free(net->ct.nf_conntrack_cachep, ct);
> > 
> > net->ct.nf_conntrack_cachep is created with SLAB_DESTROY_BY_RCU.
> > 
> > The hash is protected by rcu, so readers look up conntracks without
> > locks.
> > A conntrack is removed from the hash, but in this moment a few readers
> > still can use the conntrack. Then this conntrack is released and another
> > thread creates conntrack with the same address and the equal tuple.
> > After this a reader starts to validate the conntrack:
> > * It's not dying, because a new conntrack was created
> > * nf_ct_tuple_equal() returns true.
> ...
> 
> 
> > v2: move nf_ct_is_confirmed into the unlikely() annotation
> > v3: Eric suggested to fix refcnt, so that it becomes zero before adding
> >     in a hash, but we can't find a way how to do that. Another way is to
> >     interpret the confirm bit as part of a search key and check it in
> >     ____nf_conntrack_find() too.
> > 
> > Cc: Eric Dumazet <eric.dumazet@gmail.com>
> > Cc: Florian Westphal <fw@strlen.de>
> > Cc: Pablo Neira Ayuso <pablo@netfilter.org>
> > Cc: Patrick McHardy <kaber@trash.net>
> > Cc: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
> > Cc: "David S. Miller" <davem@davemloft.net>
> > Cc: Cyrill Gorcunov <gorcunov@openvz.org>
> > Signed-off-by: Andrey Vagin <avagin@openvz.org>
> > ---
> 
> Acked-by: Eric Dumazet <edumazet@google.com>
> 
> Thanks Andrey !
> 

Eh, looks like this path is incomplete too:(

I think we can't set a reference counter for objects which is allocated
from a SLAB_DESTROY_BY_RCU cache. Look at the following backtrace.

cpu1					cpu2
ct = ____nf_conntrack_find()
					destroy_conntrack
atomic_inc_not_zero(ct)
					__nf_conntrack_alloc
					atomic_set(&ct->ct_general.use, 1);
if (!nf_ct_key_equal(h, tuple, zone))
nf_ct_put(ct);
  destroy_conntrack(ct) !!!!
					/* continues to use the conntrack */


Did I miss something again?

I think __nf_conntrack_alloc must use atomic_inc instead of
atomic_set. And we must be sure, that the first object from a new page is
zeroized.

I am talking about this, because after this patch a bug was triggered from
another place:

<2>[67096.759353] kernel BUG at net/netfilter/nf_conntrack_core.c:211!
<4>[67096.759371] invalid opcode: 0000 [#1] SMP 
<4>[67096.759385] last sysfs file: /sys/devices/virtual/block/md0/md/metadata_version
<4>[67096.759414] CPU 2 
<4>[67096.759422] Modules linked in: xt_comment sch_sfq cls_fw sch_htb pio_nfs pio_direct pfmt_raw pfmt_ploop1 ploop simfs xt_string xt_hashlimit xt_recent xt_length xt_hl xt_tcpmss xt_TCPMSS xt_multiport xt_limit xt_dscp vzevent coretemp cpufreq_ondemand acpi_cpufreq freq_table mperf 8021q garp stp llc ipt_REJECT iptable_filter iptable_mangle xt_NOTRACK iptable_raw iptable_nat ip_tables ip6t_REJECT nf_conntrack_ipv6 nf_defrag_ipv6 xt_state ip6table_filter ip6table_raw xt_MARK ip6table_mangle ip6_tables ext4 jbd2 tun ip_gre ipip vzethdev vznetdev vzrst nf_nat nf_conntrack_ipv4 nf_defrag_ipv4 ipv6 vzcpt nf_conntrack vzdquota vzmon vziolimit vzdev tunnel4 nfsd nfs lockd fscache auth_rpcgss nfs_acl sunrpc tpm_tis tpm tpm_bios microcode serio_raw i2c_i801 sg iTCO_wdt iTCO_vendor_support e1000e
  ext3 jbd mbcache raid1 sd_mod crc_t10dif ata_piix ahci pata_acpi ata_generic i915 drm_kms_helper drm i2c_algo_bit i2c_core video output dm_mirror dm_region_hash dm_log dm_mod [last unloaded: scsi_wait_scan]
<4>[67096.759801] 
<4>[67096.759837] Pid: 498649, comm: atdd veid: 666 Tainted: G         C ---------------    2.6.32-042stab084.18 #1 042stab084_18                  /DQ45CB
<4>[67096.759932] RIP: 0010:[<ffffffffa03d99ac>]  [<ffffffffa03d99ac>] destroy_conntrack+0x15c/0x190 [nf_conntrack]
<4>[67096.760032] RSP: 0000:ffff88001ae378b8  EFLAGS: 00010246
<4>[67096.760075] RAX: 0000000000000000 RBX: ffff8801a57ac928 RCX: 0000000000065000
<4>[67096.760123] RDX: 000000000000f603 RSI: 0000000000000006 RDI: ffff8801a57ac928
<4>[67096.760174] RBP: ffff88001ae378d8 R08: 0000000000000002 R09: ffff8802373b06e0
<4>[67096.760221] R10: 0000000000000001 R11: 0000000000000000 R12: ffff88023928c080
<4>[67096.760255] R13: ffff880237e8c000 R14: 0000000000000002 R15: 0000000000000002
<4>[67096.760255] FS:  0000000000000000(0000) GS:ffff880028300000(0063) knlGS:00000000b63afbb0
<4>[67096.760255] CS:  0010 DS: 002b ES: 002b CR0: 000000008005003b
<4>[67096.760255] CR2: 00000000b74f44c0 CR3: 00000000b89c6000 CR4: 00000000000007e0
<4>[67096.760255] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
<4>[67096.760255] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
<4>[67096.760255] Process atdd (pid: 498649, veid: 666, threadinfo ffff88001ae36000, task ffff88001deaa980)
<4>[67096.760255] Stack:
<4>[67096.760255]  ffff88001ae378e8 ffff88001ae37988 ffff88023928c080 0000000000000003
<4>[67096.760255] <d> ffff88001ae378e8 ffffffff814844a7 ffff88001ae37908 ffffffffa03d9bb5
<4>[67096.760255] <d> ffff88012dcae580 ffff88023928c080 ffff88001ae379e8 ffffffffa03d9fb2
<4>[67096.760255] Call Trace:
<4>[67096.760255]  [<ffffffff814844a7>] nf_conntrack_destroy+0x17/0x30
<4>[67096.760255]  [<ffffffffa03d9bb5>] nf_conntrack_find_get+0x85/0x130 [nf_conntrack]
<4>[67096.760255]  [<ffffffffa03d9fb2>] nf_conntrack_in+0x352/0xb60 [nf_conntrack]
<4>[67096.760255]  [<ffffffffa048c771>] ipv4_conntrack_local+0x51/0x60 [nf_conntrack_ipv4]
<4>[67096.760255]  [<ffffffff81484419>] nf_iterate+0x69/0xb0
<4>[67096.760255]  [<ffffffff814b5b00>] ? dst_output+0x0/0x20
<4>[67096.760255]  [<ffffffff814845d4>] nf_hook_slow+0x74/0x110
<4>[67096.760255]  [<ffffffff814b5b00>] ? dst_output+0x0/0x20
<4>[67096.760255]  [<ffffffff814b66d5>] raw_sendmsg+0x775/0x910
<4>[67096.760255]  [<ffffffff8104c5a8>] ? flush_tlb_others_ipi+0x128/0x130
<4>[67096.760255]  [<ffffffff8100bc4e>] ? apic_timer_interrupt+0xe/0x20
<4>[67096.760255]  [<ffffffff8100bc4e>] ? apic_timer_interrupt+0xe/0x20
<4>[67096.760255]  [<ffffffff814c136a>] inet_sendmsg+0x4a/0xb0
<4>[67096.760255]  [<ffffffff81444e93>] ? sock_sendmsg+0x13/0x140
<4>[67096.760255]  [<ffffffff81444f97>] sock_sendmsg+0x117/0x140
<4>[67096.760255]  [<ffffffff8102e299>] ? native_smp_send_reschedule+0x49/0x60
<4>[67096.760255]  [<ffffffff81519beb>] ? _spin_unlock_bh+0x1b/0x20
<4>[67096.760255]  [<ffffffff8109d930>] ? autoremove_wake_function+0x0/0x40
<4>[67096.760255]  [<ffffffff814960f0>] ? do_ip_setsockopt+0x90/0xd80
<4>[67096.760255]  [<ffffffff8100bc4e>] ? apic_timer_interrupt+0xe/0x20
<4>[67096.760255]  [<ffffffff8100bc4e>] ? apic_timer_interrupt+0xe/0x20
<4>[67096.760255]  [<ffffffff814457c9>] sys_sendto+0x139/0x190
<4>[67096.760255]  [<ffffffff810efa77>] ? audit_syscall_entry+0x1d7/0x200
<4>[67096.760255]  [<ffffffff810ef7c5>] ? __audit_syscall_exit+0x265/0x290
<4>[67096.760255]  [<ffffffff81474daf>] compat_sys_socketcall+0x13f/0x210
<4>[67096.760255]  [<ffffffff8104dea3>] ia32_sysret+0x0/0x5
<4>[67096.760255] Code: 0b ab 0a e1 eb b7 f6 05 34 f8 00 e2 20 74 b7 80 3d f0 b0 00 00 00 74 ae 48 89 de 48 c7 c7 20 16 3e a0 31 c0 e8 05 ca 13 e1 eb 9b <0f> 0b eb fe f6 05 0b f8 00 e2 20 0f 84 db fe ff ff 80 3d eb b0 
<1>[67096.760255] RIP  [<ffffffffa03d99ac>] destroy_conntrack+0x15c/0x190 [nf_conntrack]
<4>[67096.760255]  RSP <ffff88001ae378b8>

^ permalink raw reply

* Re: [PATCHv3 net-next 2/5] {IPv4,xfrm} Add ESN support for AH ingress part
From: Steffen Klassert @ 2014-01-14 10:51 UTC (permalink / raw)
  To: Fan Du; +Cc: davem, netdev
In-Reply-To: <52D5144B.605@windriver.com>

On Tue, Jan 14, 2014 at 06:41:15PM +0800, Fan Du wrote:
> 
> 
> On 2014年01月14日 18:34, Steffen Klassert wrote:
> >On Tue, Jan 14, 2014 at 06:17:26PM +0800, Fan Du wrote:
> >>On 2014年01月14日 18:09, Steffen Klassert wrote:
> >
> >No, I mean something like:
> >
> >sg_init_table(sg, nfrags + sglists)
> >
> >if (x->props.flags&  XFRM_STATE_ESN) {
> >	*seqhi = XFRM_SKB_CB(skb)->seq.input.hi;
> >	sg_set_buf(sg + nfrags, seqhi, seqhi_len);
> >}
> >
> 
> hehe, it's the same as the option this patch used.

No, you don't need to sg_unmark_end() before you can add
your entry and it is more obvious what happens here.

Also, I'm not absolutely sure whether the sg magic allows
what you did. Did you test with sg debugging enabled?

> Anyway, I will make it as you suggested in the next round review.
> 

Thanks!

^ permalink raw reply

* Re: [PATCH v2] net: can: Disable flexcan driver build for big endian CPU on ARM
From: Arnd Bergmann @ 2014-01-14 10:44 UTC (permalink / raw)
  To: Marc Kleine-Budde; +Cc: Wolfgang Grandegger, linux-can, netdev

> On 01/06/2014 02:21 PM, Guenter Roeck wrote:
> > Building arm:allmodconfig fails with
> > 
> > flexcan.c: In function 'flexcan_read':
> > flexcan.c:243:2: error: implicit declaration of function 'in_be32'
> > flexcan.c: In function 'flexcan_write':
> > flexcan.c:248:2: error: implicit declaration of function 'out_be32'
> > 
> > in_be32 and out_be32 do not (or no longer) exist for ARM targets.
> > Disable the build for ARM on big endian CPUs.
> > 
> > Signed-off-by: Guenter Roeck <linux@xxxxxxxxxxxx>
> 
> Applied to can-next.

Sorry, this patch was wrong.

There is no reason to disallow building the driver on big-endian
ARM kernels. Furthermore, the current behavior is actually broken
on little-endian PowerPC as well.
The choice of register accessor functions must purely depend
on the CPU architecture, not which endianess the CPU is running
on. Note that we nowadays allow both big-endian ARM and little-endian
PowerPC kernels.
With this patch applied, we will do the right thing in all four
combinations.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index aaed97b..320bef2 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -235,9 +235,12 @@ static const struct can_bittiming_const flexcan_bittiming_const = {
 };
 
 /*
- * Abstract off the read/write for arm versus ppc.
+ * Abstract off the read/write for arm versus ppc. This
+ * assumes that PPC uses big-endian registers and everything
+ * else uses little-endian registers, independent of CPU
+ * endianess.
  */
-#if defined(__BIG_ENDIAN)
+#if defined(CONFIG_PPC)
 static inline u32 flexcan_read(void __iomem *addr)
 {
 	return in_be32(addr);


^ permalink raw reply related

* Re: [PATCHv3 net-next 2/5] {IPv4,xfrm} Add ESN support for AH ingress part
From: Fan Du @ 2014-01-14 10:41 UTC (permalink / raw)
  To: Steffen Klassert; +Cc: davem, netdev
In-Reply-To: <20140114103426.GJ31491@secunet.com>



On 2014年01月14日 18:34, Steffen Klassert wrote:
> On Tue, Jan 14, 2014 at 06:17:26PM +0800, Fan Du wrote:
>>
>>
>> On 2014年01月14日 18:09, Steffen Klassert wrote:
>>> On Tue, Jan 14, 2014 at 06:01:32PM +0800, Fan Du wrote:
>>>>
>>>>
>>>> On 2014年01月14日 17:54, Steffen Klassert wrote:
>>>>> On Tue, Jan 14, 2014 at 09:39:09AM +0800, Fan Du wrote:
>>>>>> @@ -381,7 +393,14 @@ static int ah_input(struct xfrm_state *x, struct sk_buff *skb)
>>>>>>   	sg_init_table(sg, nfrags);
>>>>>>   	skb_to_sgvec(skb, sg, 0, skb->len);
>>>>>>
>>>>>> -	ahash_request_set_crypt(req, sg, icv, skb->len);
>>>>>> +	if (x->props.flags&    XFRM_STATE_ESN) {
>>>>>> +		sg_unmark_end(&sg[nfrags - 1]);
>>>>>> +		/* Attach seqhi sg right after packet payload */
>>>>>> +		*seqhi = htonl(XFRM_SKB_CB(skb)->seq.output.hi);
>>>>>
>>>>> This is ah_input(), so you should use the high bits of the input
>>>>> sequence number here. The ipv6 patch has the same problem.
>>>>
>>>> ok, I will fix this.
>>>>
>>>>>
>>>>>> +		sg_init_table(seqhisg, sglists);
>>>>>
>>>>> Why do you add a separate SG table for this?
>>>>
>>>> It just initialize a single seqhisg, which is actually followed behind packet payload sg table.
>>>> initialized seqhisg actually mark itself as the end of sg list.
>>>>
>>>
>>> Why don't you just add this entry to the existing SG table?
>>>
>>
>> Do you mean scatterwalk_crypto_chain ?
>
> No, I mean something like:
>
> sg_init_table(sg, nfrags + sglists)
>
> if (x->props.flags&  XFRM_STATE_ESN) {
> 	*seqhi = XFRM_SKB_CB(skb)->seq.input.hi;
> 	sg_set_buf(sg + nfrags, seqhi, seqhi_len);
> }
>

hehe, it's the same as the option this patch used.
Anyway, I will make it as you suggested in the next round review.


-- 
浮沉随浪只记今朝笑

--fan

^ permalink raw reply

* Re: [PATCH net-next v2 0/2] bonding: ensure that the TSO being set on bond master
From: Ding Tianhong @ 2014-01-14 10:38 UTC (permalink / raw)
  To: Veaceslav Falico; +Cc: Jay Vosburgh, Eric Dumazet, David S. Miller, Netdev
In-Reply-To: <20140114094741.GB20066@redhat.com>

On 2014/1/14 17:47, Veaceslav Falico wrote:
> On Tue, Jan 14, 2014 at 05:00:07PM +0800, Ding Tianhong wrote:
>> The commit b0ce3508(bonding: allow TSO being set on bonding master)
>> has make the TSO being set for bond dev, but in some situation, if
>> the slave did not have the NETIF_F_SG features, the bond master will
>> miss the TSO features in netdev_fix_features because the TSO is
>> depended on SG. So I have to add SG and TSO features on bond master
>> together.
>>
>> The function netdev_add_tso_features() was only be used for bonding,
>> so no need to export it in netdevice.h, remove it and add it to bonding.
>>
>> v2: If the slave hw did not support SG features, the SG should not
>>    be forced open on master, otherwise error will occur, so modify it.
>>    Some slave may support SG but not open it yet, so the bond master
>>    could try to open it when adding the salve and make sure the TSO
>>    could be open on master.
> 
> So you're forcibly enabling SG on a slave? Usually SG is always enabled,
> unless there's a very specific reason for that - like a bug in hw/sw/fw,
> like performance improvement on specific use-cases etc. etc.
> 
> So that's a wrong thing to do - to try to enable it, if it was disabled.
> 

I don't think it is a wrong thing, just a slight changes of use, but your word
is reasonable, I will miss it.

> Nacked-by: Veaceslav Falico <vfalico@redhat.com>
> 



>>
>> Ding Tianhong (2):
>>  bonding: move the netdev_add_tso_features() to bonding
>>  bonding: try to enable SG features when adding a new slave
>>
>> drivers/net/bonding/bond_main.c | 27 ++++++++++++++++++++++++++-
>> include/linux/netdevice.h       | 10 ----------
>> 2 files changed, 26 insertions(+), 11 deletions(-)
>>
>> -- 
>> 1.8.0
>>
>>
> 
> 

^ permalink raw reply

* Re: [PATCHv3 net-next 2/5] {IPv4,xfrm} Add ESN support for AH ingress part
From: Steffen Klassert @ 2014-01-14 10:34 UTC (permalink / raw)
  To: Fan Du; +Cc: davem, netdev
In-Reply-To: <52D50EB6.3010301@windriver.com>

On Tue, Jan 14, 2014 at 06:17:26PM +0800, Fan Du wrote:
> 
> 
> On 2014年01月14日 18:09, Steffen Klassert wrote:
> >On Tue, Jan 14, 2014 at 06:01:32PM +0800, Fan Du wrote:
> >>
> >>
> >>On 2014年01月14日 17:54, Steffen Klassert wrote:
> >>>On Tue, Jan 14, 2014 at 09:39:09AM +0800, Fan Du wrote:
> >>>>@@ -381,7 +393,14 @@ static int ah_input(struct xfrm_state *x, struct sk_buff *skb)
> >>>>  	sg_init_table(sg, nfrags);
> >>>>  	skb_to_sgvec(skb, sg, 0, skb->len);
> >>>>
> >>>>-	ahash_request_set_crypt(req, sg, icv, skb->len);
> >>>>+	if (x->props.flags&   XFRM_STATE_ESN) {
> >>>>+		sg_unmark_end(&sg[nfrags - 1]);
> >>>>+		/* Attach seqhi sg right after packet payload */
> >>>>+		*seqhi = htonl(XFRM_SKB_CB(skb)->seq.output.hi);
> >>>
> >>>This is ah_input(), so you should use the high bits of the input
> >>>sequence number here. The ipv6 patch has the same problem.
> >>
> >>ok, I will fix this.
> >>
> >>>
> >>>>+		sg_init_table(seqhisg, sglists);
> >>>
> >>>Why do you add a separate SG table for this?
> >>
> >>It just initialize a single seqhisg, which is actually followed behind packet payload sg table.
> >>initialized seqhisg actually mark itself as the end of sg list.
> >>
> >
> >Why don't you just add this entry to the existing SG table?
> >
> 
> Do you mean scatterwalk_crypto_chain ?

No, I mean something like:

sg_init_table(sg, nfrags + sglists)

if (x->props.flags & XFRM_STATE_ESN) {
	*seqhi = XFRM_SKB_CB(skb)->seq.input.hi;
	sg_set_buf(sg + nfrags, seqhi, seqhi_len);
}

^ permalink raw reply

* Re: [PATCH RESEND net-next v2 0/3] bonding: fix primary problem for bonding
From: Ding Tianhong @ 2014-01-14 10:26 UTC (permalink / raw)
  To: Veaceslav Falico; +Cc: Jay Vosburgh, Netdev, David S. Miller
In-Reply-To: <20140114093720.GA20066@redhat.com>

On 2014/1/14 17:37, Veaceslav Falico wrote:
> On Tue, Jan 14, 2014 at 05:08:18PM +0800, Ding Tianhong wrote:
>> If the slave's name changed, and the bond params primary is exist,
>> the bond should deal with the situation in two ways:
>>
>> 1) If the slave was the primary slave yet, clean the primary slave
>>   and reselect active slave.
>> 2) If the slave's new name is as same as bond primary, set the slave
>>   as primary slave and reselect active slave.
>>
>> If the new primary is not matching any slave in the bond, the bond should
>> record it to params, clean the primary slave and select a new active slave.
>>
>> Update bonding.txt for primary description.
> 
> You didn't even take into accoung my previous messages.
> 
> Nacked-by: Veaceslav Falico <vfalico@redhat.com>
> 

Yes, miss it, sorry about that, add and resend later.



>>
>> Ding Tianhong (3):
>>  bonding: update the primary slave when changing slave's name
>>  bonding: clean the primary slave if there is no slave matching new
>>    primary
>>  bonding: update bonding.txt for primary description.
>>
>> Documentation/networking/bonding.txt |  3 ++-
>> drivers/net/bonding/bond_main.c      | 24 ++++++++++++++++++++++--
>> drivers/net/bonding/bond_options.c   |  6 ++++++
>> 3 files changed, 30 insertions(+), 3 deletions(-)
>>
>> -- 
>> 1.8.0
>>
>>
>>
> 
> 

^ permalink raw reply

* Re: [PATCHv3 net-next 2/5] {IPv4,xfrm} Add ESN support for AH ingress part
From: Fan Du @ 2014-01-14 10:17 UTC (permalink / raw)
  To: Steffen Klassert; +Cc: davem, netdev
In-Reply-To: <20140114100900.GI31491@secunet.com>



On 2014年01月14日 18:09, Steffen Klassert wrote:
> On Tue, Jan 14, 2014 at 06:01:32PM +0800, Fan Du wrote:
>>
>>
>> On 2014年01月14日 17:54, Steffen Klassert wrote:
>>> On Tue, Jan 14, 2014 at 09:39:09AM +0800, Fan Du wrote:
>>>> @@ -381,7 +393,14 @@ static int ah_input(struct xfrm_state *x, struct sk_buff *skb)
>>>>   	sg_init_table(sg, nfrags);
>>>>   	skb_to_sgvec(skb, sg, 0, skb->len);
>>>>
>>>> -	ahash_request_set_crypt(req, sg, icv, skb->len);
>>>> +	if (x->props.flags&   XFRM_STATE_ESN) {
>>>> +		sg_unmark_end(&sg[nfrags - 1]);
>>>> +		/* Attach seqhi sg right after packet payload */
>>>> +		*seqhi = htonl(XFRM_SKB_CB(skb)->seq.output.hi);
>>>
>>> This is ah_input(), so you should use the high bits of the input
>>> sequence number here. The ipv6 patch has the same problem.
>>
>> ok, I will fix this.
>>
>>>
>>>> +		sg_init_table(seqhisg, sglists);
>>>
>>> Why do you add a separate SG table for this?
>>
>> It just initialize a single seqhisg, which is actually followed behind packet payload sg table.
>> initialized seqhisg actually mark itself as the end of sg list.
>>
>
> Why don't you just add this entry to the existing SG table?
>

Do you mean scatterwalk_crypto_chain ?

-- 
浮沉随浪只记今朝笑

--fan

^ permalink raw reply

* Re: [PATCHv3 net-next 2/5] {IPv4,xfrm} Add ESN support for AH ingress part
From: Steffen Klassert @ 2014-01-14 10:09 UTC (permalink / raw)
  To: Fan Du; +Cc: davem, netdev
In-Reply-To: <52D50AFC.6030302@windriver.com>

On Tue, Jan 14, 2014 at 06:01:32PM +0800, Fan Du wrote:
> 
> 
> On 2014年01月14日 17:54, Steffen Klassert wrote:
> >On Tue, Jan 14, 2014 at 09:39:09AM +0800, Fan Du wrote:
> >>@@ -381,7 +393,14 @@ static int ah_input(struct xfrm_state *x, struct sk_buff *skb)
> >>  	sg_init_table(sg, nfrags);
> >>  	skb_to_sgvec(skb, sg, 0, skb->len);
> >>
> >>-	ahash_request_set_crypt(req, sg, icv, skb->len);
> >>+	if (x->props.flags&  XFRM_STATE_ESN) {
> >>+		sg_unmark_end(&sg[nfrags - 1]);
> >>+		/* Attach seqhi sg right after packet payload */
> >>+		*seqhi = htonl(XFRM_SKB_CB(skb)->seq.output.hi);
> >
> >This is ah_input(), so you should use the high bits of the input
> >sequence number here. The ipv6 patch has the same problem.
> 
> ok, I will fix this.
> 
> >
> >>+		sg_init_table(seqhisg, sglists);
> >
> >Why do you add a separate SG table for this?
> 
> It just initialize a single seqhisg, which is actually followed behind packet payload sg table.
> initialized seqhisg actually mark itself as the end of sg list.
> 

Why don't you just add this entry to the existing SG table?

^ permalink raw reply

* Re: [PATCH RFC 0/9]net: stmmac PM related fixes.
From: srinivas kandagatla @ 2014-01-14 10:05 UTC (permalink / raw)
  To: srinivas.kandagatla, Giuseppe Cavallaro, David Miller
  Cc: netdev, linux-kernel
In-Reply-To: <1384774256-10039-1-git-send-email-srinivas.kandagatla@st.com>

Hi Dave/Peppe,
Do you have any plans to take this series?

Peppe already Acked these series.

Please let me know if you want me to rebase these patches to a
particular branch.

Thanks,
srini

On 18/11/13 11:30, srinivas.kandagatla@st.com wrote:
> From: Srinivas Kandagatla <srinivas.kandagatla@st.com>
> 
> Hi Peppe,
> 
> During PM_SUSPEND_FREEZE testing, I have noticed that PM support in STMMAC is
> partly broken. I had to re-arrange the code to do PM correctly. There were lot
> of things I did not like personally and some bits did not work in the first
> place. I thought this is the nice opportunity to clean the mess up.
> 
> Here is what I did:
>  any
> 1> Test PM suspend freeeze via pm_test
> It did not work for following reasons.
>  - If the power to gmac is removed when it enters in low power state.
> stmmac_resume could not cope up with such behaviour, it was expecting the ip
> register contents to be still same as before entering low power, This
> assumption is wrong. So I started to add some code to do Hardware
> initialization, thats when I started to re-arrange the code. stmmac_open
> contains both resource and memory allocations and hardware initialization. I
> had to separate these two things in two different functions.
> 
> These two patches do that
>   net: stmmac: move dma allocation to new function
>   net: stmmac: move hardware setup for stmmac_open to new function
> 
> And rest of the other patches are fixing the loose ends, things like mdio
> reset, which might be necessary in cases likes hibernation(I did not test).
> 
> In hibernation cases the driver was just unregistering with subsystems and
> releasing resources which I did not like and its not necessary to do this as
> part of PM. So using the same stmmac_suspend/resume made more sense for
> hibernation cases than using stmmac_open/release.
> Also fixed a NULL pointer dereference bug too.
> 
> 2> Test WOL via PM_SUSPEND_FREEZE
> Did get an wakeup interrupt, but could not wakeup a freeze system.
> So I had to add pm_wakeup_event to the driver.
> net: stmmac: notify the PM core of a wakeup event. patch.
> 
> Also few patches like 
>   net: stmmac: make stmmac_mdio_reset non-static
>   net: stmmac: restore pinstate in pm resume.
> helps the resume function to reset the phy and put back the pins in default
> state.
> 
> Comments?
> 
> Thanks,
> srini
> 
> Srinivas Kandagatla (9):
>   net: stmmac: support max-speed device tree property
>   net: stmmac: mdio: remove reset gpio free
>   net: stmmac: move dma allocation to new function
>   net: stmmac: move hardware setup for stmmac_open to new function
>   net: stmmac: make stmmac_mdio_reset non-static
>   net: stmmac: fix power mangement suspend-resume case
>   net: stmmac: use suspend functions for hibernation
>   net: stmmac: restore pinstate in pm resume.
>   net: stmmac: notify the PM core of a wakeup event.
> 
>  drivers/net/ethernet/stmicro/stmmac/stmmac.h       |    4 +-
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c  |  360 ++++++++++----------
>  drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c  |    3 +-
>  .../net/ethernet/stmicro/stmmac/stmmac_platform.c  |   51 +--
>  include/linux/stmmac.h                             |    1 +
>  5 files changed, 209 insertions(+), 210 deletions(-)
> 

^ permalink raw reply

* Re: [PATCHv3 net-next 2/5] {IPv4,xfrm} Add ESN support for AH ingress part
From: Fan Du @ 2014-01-14 10:01 UTC (permalink / raw)
  To: Steffen Klassert; +Cc: davem, netdev
In-Reply-To: <20140114095425.GH31491@secunet.com>



On 2014年01月14日 17:54, Steffen Klassert wrote:
> On Tue, Jan 14, 2014 at 09:39:09AM +0800, Fan Du wrote:
>> @@ -381,7 +393,14 @@ static int ah_input(struct xfrm_state *x, struct sk_buff *skb)
>>   	sg_init_table(sg, nfrags);
>>   	skb_to_sgvec(skb, sg, 0, skb->len);
>>
>> -	ahash_request_set_crypt(req, sg, icv, skb->len);
>> +	if (x->props.flags&  XFRM_STATE_ESN) {
>> +		sg_unmark_end(&sg[nfrags - 1]);
>> +		/* Attach seqhi sg right after packet payload */
>> +		*seqhi = htonl(XFRM_SKB_CB(skb)->seq.output.hi);
>
> This is ah_input(), so you should use the high bits of the input
> sequence number here. The ipv6 patch has the same problem.

ok, I will fix this.

>
>> +		sg_init_table(seqhisg, sglists);
>
> Why do you add a separate SG table for this?

It just initialize a single seqhisg, which is actually followed behind packet payload sg table.
initialized seqhisg actually mark itself as the end of sg list.

-- 
浮沉随浪只记今朝笑

--fan

^ permalink raw reply

* Re: [PATCHv3 net-next 2/5] {IPv4,xfrm} Add ESN support for AH ingress part
From: Steffen Klassert @ 2014-01-14  9:54 UTC (permalink / raw)
  To: Fan Du; +Cc: davem, netdev
In-Reply-To: <1389663552-29638-3-git-send-email-fan.du@windriver.com>

On Tue, Jan 14, 2014 at 09:39:09AM +0800, Fan Du wrote:
> @@ -381,7 +393,14 @@ static int ah_input(struct xfrm_state *x, struct sk_buff *skb)
>  	sg_init_table(sg, nfrags);
>  	skb_to_sgvec(skb, sg, 0, skb->len);
>  
> -	ahash_request_set_crypt(req, sg, icv, skb->len);
> +	if (x->props.flags & XFRM_STATE_ESN) {
> +		sg_unmark_end(&sg[nfrags - 1]);
> +		/* Attach seqhi sg right after packet payload */
> +		*seqhi = htonl(XFRM_SKB_CB(skb)->seq.output.hi);

This is ah_input(), so you should use the high bits of the input
sequence number here. The ipv6 patch has the same problem.

> +		sg_init_table(seqhisg, sglists);

Why do you add a separate SG table for this?

^ permalink raw reply

* Re: [PATCH net-next] tun/macvtap: limit the packets queued through rcvbuf
From: Michael S. Tsirkin @ 2014-01-14  9:52 UTC (permalink / raw)
  To: Jason Wang
  Cc: davem, netdev, linux-kernel, Vlad Yasevich, John Fastabend,
	Stephen Hemminger, Herbert Xu
In-Reply-To: <52D4F924.2040502@redhat.com>

On Tue, Jan 14, 2014 at 04:45:24PM +0800, Jason Wang wrote:
> On 01/14/2014 04:25 PM, Michael S. Tsirkin wrote:
> > On Tue, Jan 14, 2014 at 02:53:07PM +0800, Jason Wang wrote:
> >> We used to limit the number of packets queued through tx_queue_length. This
> >> has several issues:
> >>
> >> - tx_queue_length is the control of qdisc queue length, simply reusing it
> >>   to control the packets queued by device may cause confusion.
> >> - After commit 6acf54f1cf0a6747bac9fea26f34cfc5a9029523 ("macvtap: Add
> >>   support of packet capture on macvtap device."), an unexpected qdisc
> >>   caused by non-zero tx_queue_length will lead qdisc lock contention for
> >>   multiqueue deivce.
> >> - What we really want is to limit the total amount of memory occupied not
> >>   the number of packets.
> >>
> >> So this patch tries to solve the above issues by using socket rcvbuf to
> >> limit the packets could be queued for tun/macvtap. This was done by using
> >> sock_queue_rcv_skb() instead of a direct call to skb_queue_tail(). Also two
> >> new ioctl() were introduced for userspace to change the rcvbuf like what we
> >> have done for sndbuf.
> >>
> >> With this fix, we can safely change the tx_queue_len of macvtap to
> >> zero. This will make multiqueue works without extra lock contention.
> >>
> >> Cc: Vlad Yasevich <vyasevic@redhat.com>
> >> Cc: Michael S. Tsirkin <mst@redhat.com>
> >> Cc: John Fastabend <john.r.fastabend@intel.com>
> >> Cc: Stephen Hemminger <stephen@networkplumber.org>
> >> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> >> Signed-off-by: Jason Wang <jasowang@redhat.com>
> > No, I don't think we can change userspace-visible behaviour like that.
> >
> > This will break any existing user that tries to control
> > queue length through sysfs,netlink or device ioctl.
> 
> But it looks like a buggy API, since tx_queue_len should be for qdisc
> queue length instead of device itself.

Probably, but it's been like this since 2.6.x time.
Also, qdisc queue is unused for tun so it seemed kind of
reasonable to override tx_queue_len.

> If we really want to preserve the
> behaviour, how about using a new feature flag and change the behaviour
> only when the device is created (TUNSETIFF) with the new flag?

OK this addresses the issue partially, but there's also an issue
of permissions: tx_queue_len can only be changed if
capable(CAP_NET_ADMIN). OTOH in your patch a regular user
can change the amount of memory consumed per queue
by calling TUNSETRCVBUF.

> >
> > Take a look at my patch in msg ID 20140109071721.GD19559@redhat.com
> > which gives one way to set tx_queue_len to zero without
> > breaking userspace.
> 
> If I read the patch correctly, it will make no way for the user who
> really want to change the qdisc queue length for tun.

Why would this matter?  As far as I can see qdisc queue is currently unused.

> >
> >
> >> ---
> >>  drivers/net/macvtap.c       | 31 ++++++++++++++++++++---------
> >>  drivers/net/tun.c           | 48 +++++++++++++++++++++++++++++++++------------
> >>  include/uapi/linux/if_tun.h |  3 +++
> >>  3 files changed, 60 insertions(+), 22 deletions(-)
> >>
> >> diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
> >> index a2c3a89..c429c56 100644
> >> --- a/drivers/net/macvtap.c
> >> +++ b/drivers/net/macvtap.c
> >> @@ -292,9 +292,6 @@ static rx_handler_result_t macvtap_handle_frame(struct sk_buff **pskb)
> >>  	if (!q)
> >>  		return RX_HANDLER_PASS;
> >>  
> >> -	if (skb_queue_len(&q->sk.sk_receive_queue) >= dev->tx_queue_len)
> >> -		goto drop;
> >> -
> >>  	skb_push(skb, ETH_HLEN);
> >>  
> >>  	/* Apply the forward feature mask so that we perform segmentation
> >> @@ -310,8 +307,10 @@ static rx_handler_result_t macvtap_handle_frame(struct sk_buff **pskb)
> >>  			goto drop;
> >>  
> >>  		if (!segs) {
> >> -			skb_queue_tail(&q->sk.sk_receive_queue, skb);
> >> -			goto wake_up;
> >> +			if (sock_queue_rcv_skb(&q->sk, skb))
> >> +				goto drop;
> >> +			else
> >> +				goto wake_up;
> >>  		}
> >>  
> >>  		kfree_skb(skb);
> >> @@ -319,11 +318,17 @@ static rx_handler_result_t macvtap_handle_frame(struct sk_buff **pskb)
> >>  			struct sk_buff *nskb = segs->next;
> >>  
> >>  			segs->next = NULL;
> >> -			skb_queue_tail(&q->sk.sk_receive_queue, segs);
> >> +			if (sock_queue_rcv_skb(&q->sk, segs)) {
> >> +				skb = segs;
> >> +				skb->next = nskb;
> >> +				goto drop;
> >> +			}
> >> +
> >>  			segs = nskb;
> >>  		}
> >>  	} else {
> >> -		skb_queue_tail(&q->sk.sk_receive_queue, skb);
> >> +		if (sock_queue_rcv_skb(&q->sk, skb))
> >> +			goto drop;
> >>  	}
> >>  
> >>  wake_up:
> >> @@ -333,7 +338,7 @@ wake_up:
> >>  drop:
> >>  	/* Count errors/drops only here, thus don't care about args. */
> >>  	macvlan_count_rx(vlan, 0, 0, 0);
> >> -	kfree_skb(skb);
> >> +	kfree_skb_list(skb);
> >>  	return RX_HANDLER_CONSUMED;
> >>  }
> >>  
> >> @@ -414,7 +419,7 @@ static void macvtap_dellink(struct net_device *dev,
> >>  static void macvtap_setup(struct net_device *dev)
> >>  {
> >>  	macvlan_common_setup(dev);
> >> -	dev->tx_queue_len = TUN_READQ_SIZE;
> >> +	dev->tx_queue_len = 0;
> >>  }
> >>  
> >>  static struct rtnl_link_ops macvtap_link_ops __read_mostly = {
> >> @@ -469,6 +474,7 @@ static int macvtap_open(struct inode *inode, struct file *file)
> >>  	sock_init_data(&q->sock, &q->sk);
> >>  	q->sk.sk_write_space = macvtap_sock_write_space;
> >>  	q->sk.sk_destruct = macvtap_sock_destruct;
> >> +	q->sk.sk_rcvbuf = TUN_RCVBUF;
> >>  	q->flags = IFF_VNET_HDR | IFF_NO_PI | IFF_TAP;
> >>  	q->vnet_hdr_sz = sizeof(struct virtio_net_hdr);
> >>  
> >> @@ -1040,6 +1046,13 @@ static long macvtap_ioctl(struct file *file, unsigned int cmd,
> >>  		q->sk.sk_sndbuf = u;
> >>  		return 0;
> >>  
> >> +	case TUNSETRCVBUF:
> >> +		if (get_user(u, up))
> >> +			return -EFAULT;
> >> +
> >> +		q->sk.sk_rcvbuf = u;
> >> +		return 0;
> >> +
> >>  	case TUNGETVNETHDRSZ:
> >>  		s = q->vnet_hdr_sz;
> >>  		if (put_user(s, sp))
> >> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> >> index 09f6662..7a08fa3 100644
> >> --- a/drivers/net/tun.c
> >> +++ b/drivers/net/tun.c
> >> @@ -177,6 +177,7 @@ struct tun_struct {
> >>  
> >>  	int			vnet_hdr_sz;
> >>  	int			sndbuf;
> >> +	int			rcvbuf;
> >>  	struct tap_filter	txflt;
> >>  	struct sock_fprog	fprog;
> >>  	/* protected by rtnl lock */
> >> @@ -771,17 +772,6 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
> >>  	if (!check_filter(&tun->txflt, skb))
> >>  		goto drop;
> >>  
> >> -	if (tfile->socket.sk->sk_filter &&
> >> -	    sk_filter(tfile->socket.sk, skb))
> >> -		goto drop;
> >> -
> >> -	/* Limit the number of packets queued by dividing txq length with the
> >> -	 * number of queues.
> >> -	 */
> >> -	if (skb_queue_len(&tfile->socket.sk->sk_receive_queue)
> >> -			  >= dev->tx_queue_len / tun->numqueues)
> >> -		goto drop;
> >> -
> >>  	if (unlikely(skb_orphan_frags(skb, GFP_ATOMIC)))
> >>  		goto drop;
> >>  
> >> @@ -798,7 +788,8 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
> >>  	nf_reset(skb);
> >>  
> >>  	/* Enqueue packet */
> >> -	skb_queue_tail(&tfile->socket.sk->sk_receive_queue, skb);
> >> +	if (sock_queue_rcv_skb(tfile->socket.sk, skb))
> >> +		goto drop;
> >>  
> >>  	/* Notify and wake up reader process */
> >>  	if (tfile->flags & TUN_FASYNC)
> >> @@ -1668,6 +1659,7 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
> >>  
> >>  		tun->filter_attached = false;
> >>  		tun->sndbuf = tfile->socket.sk->sk_sndbuf;
> >> +		tun->rcvbuf = tfile->socket.sk->sk_rcvbuf;
> >>  
> >>  		spin_lock_init(&tun->lock);
> >>  
> >> @@ -1837,6 +1829,17 @@ static void tun_set_sndbuf(struct tun_struct *tun)
> >>  	}
> >>  }
> >>  
> >> +static void tun_set_rcvbuf(struct tun_struct *tun)
> >> +{
> >> +	struct tun_file *tfile;
> >> +	int i;
> >> +
> >> +	for (i = 0; i < tun->numqueues; i++) {
> >> +		tfile = rtnl_dereference(tun->tfiles[i]);
> >> +		tfile->socket.sk->sk_sndbuf = tun->sndbuf;
> >> +	}
> >> +}
> >> +
> >>  static int tun_set_queue(struct file *file, struct ifreq *ifr)
> >>  {
> >>  	struct tun_file *tfile = file->private_data;
> >> @@ -1878,7 +1881,7 @@ static long __tun_chr_ioctl(struct file *file, unsigned int cmd,
> >>  	struct ifreq ifr;
> >>  	kuid_t owner;
> >>  	kgid_t group;
> >> -	int sndbuf;
> >> +	int sndbuf, rcvbuf;
> >>  	int vnet_hdr_sz;
> >>  	unsigned int ifindex;
> >>  	int ret;
> >> @@ -2061,6 +2064,22 @@ static long __tun_chr_ioctl(struct file *file, unsigned int cmd,
> >>  		tun_set_sndbuf(tun);
> >>  		break;
> >>  
> >> +	case TUNGETRCVBUF:
> >> +		rcvbuf = tfile->socket.sk->sk_rcvbuf;
> >> +		if (copy_to_user(argp, &rcvbuf, sizeof(rcvbuf)))
> >> +			ret = -EFAULT;
> >> +		break;
> >> +
> >> +	case TUNSETRCVBUF:
> >> +		if (copy_from_user(&rcvbuf, argp, sizeof(rcvbuf))) {
> >> +			ret = -EFAULT;
> >> +			break;
> >> +		}
> >> +
> >> +		tun->rcvbuf = rcvbuf;
> >> +		tun_set_rcvbuf(tun);
> >> +		break;
> >> +
> >>  	case TUNGETVNETHDRSZ:
> >>  		vnet_hdr_sz = tun->vnet_hdr_sz;
> >>  		if (copy_to_user(argp, &vnet_hdr_sz, sizeof(vnet_hdr_sz)))
> >> @@ -2139,6 +2158,8 @@ static long tun_chr_compat_ioctl(struct file *file,
> >>  	case TUNSETTXFILTER:
> >>  	case TUNGETSNDBUF:
> >>  	case TUNSETSNDBUF:
> >> +	case TUNGETRCVBUF:
> >> +	case TUNSETRCVBUF:
> >>  	case SIOCGIFHWADDR:
> >>  	case SIOCSIFHWADDR:
> >>  		arg = (unsigned long)compat_ptr(arg);
> >> @@ -2204,6 +2225,7 @@ static int tun_chr_open(struct inode *inode, struct file * file)
> >>  
> >>  	tfile->sk.sk_write_space = tun_sock_write_space;
> >>  	tfile->sk.sk_sndbuf = INT_MAX;
> >> +	tfile->sk.sk_rcvbuf = TUN_RCVBUF;
> >>  
> >>  	file->private_data = tfile;
> >>  	set_bit(SOCK_EXTERNALLY_ALLOCATED, &tfile->socket.flags);
> >> diff --git a/include/uapi/linux/if_tun.h b/include/uapi/linux/if_tun.h
> >> index e9502dd..8e04657 100644
> >> --- a/include/uapi/linux/if_tun.h
> >> +++ b/include/uapi/linux/if_tun.h
> >> @@ -22,6 +22,7 @@
> >>  
> >>  /* Read queue size */
> >>  #define TUN_READQ_SIZE	500
> >> +#define TUN_RCVBUF	(512 * PAGE_SIZE)
> >>  
> >>  /* TUN device flags */
> >>  #define TUN_TUN_DEV 	0x0001	
> > That's about 16x less than we were able to queue previously
> > by default.
> > How can you be sure this won't break any applications?
> >
> 
> Ok, we can change it back to 500 * 65535, but I'm not sure whether this
> or not this value (about 32M) is too big for a single socket.
> >> @@ -58,6 +59,8 @@
> >>  #define TUNSETQUEUE  _IOW('T', 217, int)
> >>  #define TUNSETIFINDEX	_IOW('T', 218, unsigned int)
> >>  #define TUNGETFILTER _IOR('T', 219, struct sock_fprog)
> >> +#define TUNGETRCVBUF   _IOR('T', 220, int)
> >> +#define TUNSETRCVBUF   _IOW('T', 221, int)
> >>  
> >>  /* TUNSETIFF ifr flags */
> >>  #define IFF_TUN		0x0001
> >> -- 
> >> 1.8.3.2
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply

* Re: [PATCH net-next v2 0/2] bonding: ensure that the TSO being set on bond master
From: Veaceslav Falico @ 2014-01-14  9:47 UTC (permalink / raw)
  To: Ding Tianhong; +Cc: Jay Vosburgh, Eric Dumazet, David S. Miller, Netdev
In-Reply-To: <52D4FC97.8020301@huawei.com>

On Tue, Jan 14, 2014 at 05:00:07PM +0800, Ding Tianhong wrote:
>The commit b0ce3508(bonding: allow TSO being set on bonding master)
>has make the TSO being set for bond dev, but in some situation, if
>the slave did not have the NETIF_F_SG features, the bond master will
>miss the TSO features in netdev_fix_features because the TSO is
>depended on SG. So I have to add SG and TSO features on bond master
>together.
>
>The function netdev_add_tso_features() was only be used for bonding,
>so no need to export it in netdevice.h, remove it and add it to bonding.
>
>v2: If the slave hw did not support SG features, the SG should not
>    be forced open on master, otherwise error will occur, so modify it.
>    Some slave may support SG but not open it yet, so the bond master
>    could try to open it when adding the salve and make sure the TSO
>    could be open on master.

So you're forcibly enabling SG on a slave? Usually SG is always enabled,
unless there's a very specific reason for that - like a bug in hw/sw/fw,
like performance improvement on specific use-cases etc. etc.

So that's a wrong thing to do - to try to enable it, if it was disabled.

Nacked-by: Veaceslav Falico <vfalico@redhat.com>

>
>Ding Tianhong (2):
>  bonding: move the netdev_add_tso_features() to bonding
>  bonding: try to enable SG features when adding a new slave
>
> drivers/net/bonding/bond_main.c | 27 ++++++++++++++++++++++++++-
> include/linux/netdevice.h       | 10 ----------
> 2 files changed, 26 insertions(+), 11 deletions(-)
>
>-- 
>1.8.0
>
>

^ permalink raw reply

* Re: [GIT net-next] Open vSwitch
From: Thomas Graf @ 2014-01-14  9:46 UTC (permalink / raw)
  To: Jesse Gross, Zoltan Kiss
  Cc: dev-yBygre7rU0TnMu66kgdUjQ@public.gmane.org, netdev, David Miller
In-Reply-To: <CAEP_g=8nG6AHV9Y+5=48nPhkf5Oe=mG8EiyaKSqN4omnmGhv4A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On 01/14/2014 02:30 AM, Jesse Gross wrote:
>> And it works. I guess the last one causing the problem. Might be an
>> important factor, I'm using 32 bit Dom0.
>
> I think you're probably right. Thomas - can you take a look?
>
> We shouldn't be doing any zerocopy in this situation but it looks to
> me like we don't do any padding at all, even in situations where we
> are copying the data.

I'm looking into this now. The zerocopy method should only be attempted
if user space has announced the ability to received unaligned messages.

@Zoltan: I assume you are using an unmodified OVS 1.9.3?

^ permalink raw reply

* Re: [PATCH RESEND net-next v2 0/3] bonding: fix primary problem for bonding
From: Veaceslav Falico @ 2014-01-14  9:37 UTC (permalink / raw)
  To: Ding Tianhong; +Cc: Jay Vosburgh, Netdev, David S. Miller
In-Reply-To: <52D4FE82.8020507@huawei.com>

On Tue, Jan 14, 2014 at 05:08:18PM +0800, Ding Tianhong wrote:
>If the slave's name changed, and the bond params primary is exist,
>the bond should deal with the situation in two ways:
>
>1) If the slave was the primary slave yet, clean the primary slave
>   and reselect active slave.
>2) If the slave's new name is as same as bond primary, set the slave
>   as primary slave and reselect active slave.
>
>If the new primary is not matching any slave in the bond, the bond should
>record it to params, clean the primary slave and select a new active slave.
>
>Update bonding.txt for primary description.

You didn't even take into accoung my previous messages.

Nacked-by: Veaceslav Falico <vfalico@redhat.com>

>
>Ding Tianhong (3):
>  bonding: update the primary slave when changing slave's name
>  bonding: clean the primary slave if there is no slave matching new
>    primary
>  bonding: update bonding.txt for primary description.
>
> Documentation/networking/bonding.txt |  3 ++-
> drivers/net/bonding/bond_main.c      | 24 ++++++++++++++++++++++--
> drivers/net/bonding/bond_options.c   |  6 ++++++
> 3 files changed, 30 insertions(+), 3 deletions(-)
>
>-- 
>1.8.0
>
>
>

^ permalink raw reply

* Re: [Xen-devel] [PATCH net-next v2 3/3] xen-netfront: use new skb_checksum_setup function
From: Ian Campbell @ 2014-01-14  9:33 UTC (permalink / raw)
  To: David Miller
  Cc: paul.durrant, netdev, boris.ostrovsky, david.vrabel, xen-devel
In-Reply-To: <20140113.112720.2236966361404094251.davem@davemloft.net>

On Mon, 2014-01-13 at 11:27 -0800, David Miller wrote:
> From: Paul Durrant <paul.durrant@citrix.com>
> Date: Thu, 9 Jan 2014 10:02:48 +0000
> 
> > Use skb_checksum_setup to set up partial checksum offsets rather
> > then a private implementation.
> > 
> > Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> 
> This patch really needs review by a netfront expert before I can apply
> this series.

I think in combination with 1/3 it is basically code motion. So although
I'm not not netfront maintainer:

Acked-by: Ian Campbell <ian.campbell@citrix.com>

Ian.

^ permalink raw reply

* [PATCH RESEND net-next v2 3/3] bonding: update bonding.txt for primary description
From: Ding Tianhong @ 2014-01-14  9:08 UTC (permalink / raw)
  To: Jay Vosburgh, Veaceslav Falico, David S. Miller, Netdev

Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
 Documentation/networking/bonding.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/networking/bonding.txt b/Documentation/networking/bonding.txt
index a4d925e..5cdb229 100644
--- a/Documentation/networking/bonding.txt
+++ b/Documentation/networking/bonding.txt
@@ -657,7 +657,8 @@ primary
 	one slave is preferred over another, e.g., when one slave has
 	higher throughput than another.
 
-	The primary option is only valid for active-backup mode.
+	The primary option is only valid for active-backup(1),
+	balance-tlb (5) and balance-alb (6) mode.
 
 primary_reselect
 
-- 
1.8.0

^ permalink raw reply related

* Re: [PATCH net-next v2 1/3] [PATCH 1/3] bonding: update the primary slave when changing slave's name
From: Ding Tianhong @ 2014-01-14  9:07 UTC (permalink / raw)
  To: Jay Vosburgh, Veaceslav Falico, David S. Miller, Netdev
In-Reply-To: <52D4F845.6070106@huawei.com>

On 2014/1/14 16:41, Ding Tianhong wrote:
> If the slave's name changed, and the bond params primary is exist,
> the bond should deal with the situation in two ways:
> 
> 1) If the slave was the primary slave yet, clean the primary slave
>    and reselect active slave.
> 2) If the slave's new name is as same as bond primary, set the slave
>    as primary slave and reselect active slave.
> 
> Thanks for Veaceslav's suggestion.
> 
> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>

Someting wrong in format, miss it, sorry.

Ding

> ---
>  drivers/net/bonding/bond_main.c | 24 ++++++++++++++++++++++--
>  1 file changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index e06c445..64e25d5 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -2860,9 +2860,29 @@ static int bond_slave_netdev_event(unsigned long event,
>  		 */
>  		break;
>  	case NETDEV_CHANGENAME:
> -		/*
> -		 * TODO: handle changing the primary's name
> +		/* Handle changing the slave's name:
> +		 * 1) If the slave was primary slave,
> +		 * clean the primary slave and reselect
> +		 * active slave.
> +		 * 2) If the slave's new name is same as
> +		 * bond primary, set the slave as primary
> +		 * slave and reselect active slave.
>  		 */
> +		if (slave == bond->primary_slave ||
> +		    !strcmp(bond->params.primary, slave_dev->name)) {
> +			if (bond->primary_slave) {
> +				pr_info("%s: Setting primary slave to None.\n",
> +					bond->dev->name);
> +				bond->primary_slave = NULL;
> +			} else {
> +				pr_info("%s: Setting %s as primary slave.\n",
> +					bond->dev->name, slave_dev->name);
> +				bond->primary_slave = slave;
> +			}
> +			write_lock_bh(&bond->curr_slave_lock);
> +			bond_select_active_slave(bond);
> +			write_unlock_bh(&bond->curr_slave_lock);
> +		}
>  		break;
>  	case NETDEV_FEAT_CHANGE:
>  		bond_compute_features(bond);
> 

^ permalink raw reply

* [PATCH RESEND net-next v2 0/3] bonding: fix primary problem for bonding
From: Ding Tianhong @ 2014-01-14  9:08 UTC (permalink / raw)
  To: Jay Vosburgh, Veaceslav Falico, Netdev, David S. Miller

If the slave's name changed, and the bond params primary is exist,
the bond should deal with the situation in two ways:

1) If the slave was the primary slave yet, clean the primary slave
   and reselect active slave.
2) If the slave's new name is as same as bond primary, set the slave
   as primary slave and reselect active slave.

If the new primary is not matching any slave in the bond, the bond should
record it to params, clean the primary slave and select a new active slave.

Update bonding.txt for primary description.

Ding Tianhong (3):
  bonding: update the primary slave when changing slave's name
  bonding: clean the primary slave if there is no slave matching new
    primary
  bonding: update bonding.txt for primary description.

 Documentation/networking/bonding.txt |  3 ++-
 drivers/net/bonding/bond_main.c      | 24 ++++++++++++++++++++++--
 drivers/net/bonding/bond_options.c   |  6 ++++++
 3 files changed, 30 insertions(+), 3 deletions(-)

-- 
1.8.0

^ permalink raw reply

* [PATCH RESEND net-next v2 1/3] bonding: update the primary slave when changing slave's name
From: Ding Tianhong @ 2014-01-14  9:08 UTC (permalink / raw)
  To: Jay Vosburgh, Veaceslav Falico, David S. Miller, Netdev

If the slave's name changed, and the bond params primary is exist,
the bond should deal with the situation in two ways:

1) If the slave was the primary slave yet, clean the primary slave
   and reselect active slave.
2) If the slave's new name is as same as bond primary, set the slave
   as primary slave and reselect active slave.

Thanks for Veaceslav's suggestion.

Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
 drivers/net/bonding/bond_main.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index e06c445..64e25d5 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2860,9 +2860,29 @@ static int bond_slave_netdev_event(unsigned long event,
 		 */
 		break;
 	case NETDEV_CHANGENAME:
-		/*
-		 * TODO: handle changing the primary's name
+		/* Handle changing the slave's name:
+		 * 1) If the slave was primary slave,
+		 * clean the primary slave and reselect
+		 * active slave.
+		 * 2) If the slave's new name is same as
+		 * bond primary, set the slave as primary
+		 * slave and reselect active slave.
 		 */
+		if (slave == bond->primary_slave ||
+		    !strcmp(bond->params.primary, slave_dev->name)) {
+			if (bond->primary_slave) {
+				pr_info("%s: Setting primary slave to None.\n",
+					bond->dev->name);
+				bond->primary_slave = NULL;
+			} else {
+				pr_info("%s: Setting %s as primary slave.\n",
+					bond->dev->name, slave_dev->name);
+				bond->primary_slave = slave;
+			}
+			write_lock_bh(&bond->curr_slave_lock);
+			bond_select_active_slave(bond);
+			write_unlock_bh(&bond->curr_slave_lock);
+		}
 		break;
 	case NETDEV_FEAT_CHANGE:
 		bond_compute_features(bond);
-- 
1.8.0

^ permalink raw reply related

* [PATCH RESEND net-next v2 2/3] bonding: clean the primary slave if there is no slave matching new primary
From: Ding Tianhong @ 2014-01-14  9:08 UTC (permalink / raw)
  To: Jay Vosburgh, Veaceslav Falico, David S. Miller, Netdev

If the new primay is not matching any slave in the bond, the bond should
record it to params, clean the primary slave and select a new active slave.

Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
 drivers/net/bonding/bond_options.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index 945a666..0ee0bfe 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -512,6 +512,12 @@ int bond_option_primary_set(struct bonding *bond, const char *primary)
 		}
 	}
 
+	if (bond->primary_slave) {
+		pr_info("%s: Setting primary slave to None.\n",
+			bond->dev->name);
+		bond->primary_slave = NULL;
+		bond_select_active_slave(bond);
+	}
 	strncpy(bond->params.primary, primary, IFNAMSIZ);
 	bond->params.primary[IFNAMSIZ - 1] = 0;
 
-- 
1.8.0

^ 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