Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH 0/3] Fix problem with static_key decrement
From: Glauber Costa @ 2012-04-20 15:01 UTC (permalink / raw)
  To: Tejun Heo
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA, cgroups-u79uwXL29TY76Z2rM5mHXA,
	Li Zefan, kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A, David Miller,
	devel-GEFAQzZX7r8dnm+yROfE0A
In-Reply-To: <20120419225441.GD10553-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>

On 04/19/2012 07:54 PM, Tejun Heo wrote:
> On Thu, Apr 19, 2012 at 07:49:15PM -0300, Glauber Costa wrote:
>> Hi,
>>
>> This is my proposed fix for the sock memcg static_key
>> problem raised by Kamezawa. It works for me, but I would
>> Kame, please confirm.
>
> Please detail the problem.  I don't follow what's the purpose here.
>

Ok.

1) Kame found the following bug: we were decrementing the jump label 
when the socket limit was set back to unlimited. The problem is that the 
sockets outlive the memcg, so we can only do that when the last 
reference count is dropped. It is worth mentioning that kmem controller 
for memcg will have the exact same problem - I am actually updating my 
series with all the results of this discussion here.

2) If, however, there are no sockets in flight, mem_cgroup_put() during
->destroy() will be the last one, and the decrementing will happen there.

3) static_key updates cannot happen with the cgroup_mutex held. This is
because cpusets hold it from within the cpu_hotplug.lock - that 
static_keys take through get_online_cpus() in its cpu hotplug handler.

4) Looking at the cpusets code, it really seems necessary, at least by now.

5) Deferring all this to worker threads as you suggested in the cpu 
thread - that has a similar problem - can solve this problem, but in 
general, will create tons of others, like windows of inconsistent 
information.

That's basically it.

^ permalink raw reply

* Re: rx_dropped packets stop with tcpdump running
From: Brian Haley @ 2012-04-20 14:59 UTC (permalink / raw)
  To: Marco Berizzi; +Cc: tushar.n.dave, netdev, eric.dumazet
In-Reply-To: <DUB108-W45246BF22ED8BAB93B78C8B2220@phx.gbl>

On 04/20/2012 04:34 AM, Marco Berizzi wrote:
> 
> tushar.n.dave@intel.com wrote:
>> Marco,
> 
> Hi Dave,
> 
> as pointed by Eric, the counter is
> increasing because a windows box is
> sending out packets with ethertype
> 0x886d

0x886d or 0x86DD?  That second one is IPv6, you might just need to load the module.

-Brian

^ permalink raw reply

* Re: use-after-free in usbnet
From: Huajun Li @ 2012-04-20 14:56 UTC (permalink / raw)
  To: Ming Lei
  Cc: Oliver Neukum, Alan Stern, Dave Jones,
	netdev-u79uwXL29TY76Z2rM5mHXA, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	Fedora Kernel Team
In-Reply-To: <CACVXFVM5YBJkDZddeGi1_MPY7EqEV_wtoFy-NtBHYA6rxez0jg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Fri, Apr 20, 2012 at 10:22 PM, Ming Lei <tom.leiming-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Fri, Apr 20, 2012 at 9:37 PM, Huajun Li <huajun.li.lee-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>
>> Above patch has already been integrated to mainline. However, maybe
>> there still exists another potentail use-after-free issue, here is a
>> case:
>>      After release the lock in unlink_urbs(), defer_bh() may move
>> current skb from rxq/txq to dev->done queue, even cause the skb be
>> released. Then in next loop cycle, it can't refer to expected skb, and
>> may Oops again.
>
> Could you explain in a bit detail? Why can't the expected skb be refered
> to in next loop?


      unlink_urbs()                                           complete handler
--------------------------------------
-------------------------------------------------
     spin_unlock_irqrestore()
                                                                  rx_complete()
                                                                  derver_bh()

 __skb_unlink()

 __skb_queue_tail(&dev->done, skb)   =======> skb is moved to
dev->done, and can be freed by usbnet_bh()
      skb_queue_walk_safe()
                      tmp = skb->next   ===> refer to freed skb

>
>>
>> To easily reproduce it, in unlink_urbs(), you can delay a short time
>> after usb_put_urb(urb), then disconnect your device while transferring
>> data, and repeat it times you will find errors on your screen.
>
> Could you post out the error log?

The log like this:
===============================================================
[45219.230127] general protection fault: 0000 [#1] SMP DEBUG_PAGEALLOC
[45219.230192] CPU 1
[45219.230208] Modules linked in: cdc_ether usbnet(O) bluetooth
dm_crypt binfmt_misc snd_hda_codec_analog snd_hda_intel snd_hda_codec
snd_hwdep snd_pcm snd_seq_midi snd_rawmidi snd_seq_midi_event hp_wmi
ppdev sparse_keymap i915 snd_seq snd_timer coretemp microcode
snd_seq_device psmouse tpm_infineon serio_raw snd soundcore
drm_kms_helper snd_page_alloc tpm_tis tpm parport_pc tpm_bios drm
i2c_algo_bit video lp parport sg ehci_hcd uhci_hcd sr_mod sd_mod
usbcore e1000e usb_common floppy
[45219.230658]
[45219.230673] Pid: 200, comm: khubd Tainted: G          IO 3.4.0-rc3
#56 Hewlett-Packard HP Compaq dc7800p Convertible Minitower/0AACh
[45219.230761] RIP: 0010:[<ffffffffa0094e7d>]  [<ffffffffa0094e7d>]
usb_get_urb+0x1d/0x70 [usbcore]
[45219.230837] RSP: 0018:ffff8800554df8e0  EFLAGS: 00010002
[45219.230874] RAX: 6b6b6b6b6b6b6b6b RBX: 6b6b6b6b6b6b6b6b RCX: 0000000000008760
[45219.230920] RDX: 6b6b6b6b6b6b6b6b RSI: 0000000000000202 RDI: 6b6b6b6b6b6b6b6b
[45219.230966] RBP: ffff8800554df8f0 R08: 0000000304b1e1be R09: 0000000000000001
[45219.231012] R10: 0000000000000001 R11: 0000000000000000 R12: 6b6b6b6b6b6b6b6b
[45219.231058] R13: ffff88000650e330 R14: ffff88000650e318 R15: 0000000000000001
[45219.231105] FS:  0000000000000000(0000) GS:ffff88007a400000(0000)
knlGS:0000000000000000
[45219.231158] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[45219.231196] CR2: 00007f1bd331c3e0 CR3: 000000000656b000 CR4: 00000000000007e0
[45219.231243] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[45219.231289] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[45219.231335] Process khubd (pid: 200, threadinfo ffff8800554de000,
task ffff880055b4a410)
[45219.231387] Stack:
[45219.231405]  ffff8800567daf40 ffff88000650e330 ffff8800554df940
ffffffffa053ee1a
[45219.231469]  0000000000000202 ffff88000650e2a0 ffff8800554df940
ffff88000650e140
[45219.231534]  ffff880055b4a410 ffff88000650e318 ffff88000650e378
ffff88000650e2a0
[45219.231597] Call Trace:
[45219.231622]  [<ffffffffa053ee1a>] unlink_urbs.isra.14+0xca/0x170 [usbnet]
[45219.231670]  [<ffffffffa053f05b>] usbnet_terminate_urbs+0x19b/0x350 [usbnet]
[45219.231721]  [<ffffffff810d1700>] ? try_to_wake_up+0x580/0x580
[45219.231762]  [<ffffffff810c24ba>] ? __wake_up+0x3a/0x90
[45219.231801]  [<ffffffffa053ff90>] usbnet_stop+0x190/0x270 [usbnet]
[45219.231846]  [<ffffffff817b1226>] __dev_close_many+0xd6/0x160
[45219.231886]  [<ffffffff817b1368>] dev_close_many+0xb8/0x160
[45219.231926]  [<ffffffff817b1518>] rollback_registered_many+0x108/0x390
[45219.231972]  [<ffffffff817b1874>] rollback_registered+0x44/0x70
[45219.232013]  [<ffffffff817b1920>] unregister_netdevice_queue+0x80/0xf0
[45219.232058]  [<ffffffff817b1ae0>] unregister_netdev+0x30/0x50
[45219.232100]  [<ffffffffa053e61a>] usbnet_disconnect+0x8a/0x170 [usbnet]
[45219.232155]  [<ffffffffa009adc5>] usb_unbind_interface+0x65/0x230 [usbcore]
[45219.232205]  [<ffffffff8163cda7>] __device_release_driver+0x157/0x170
[45219.232249]  [<ffffffff8163cdfe>] device_release_driver+0x3e/0x60
[45219.232291]  [<ffffffff8163c27f>] bus_remove_device+0x15f/0x210
[45219.232331]  [<ffffffff81637e6d>] device_del+0x1ad/0x2b0
[45219.232379]  [<ffffffffa0097970>] usb_disable_device+0x100/0x340 [usbcore]
[45219.232435]  [<ffffffffa008a417>] usb_disconnect+0xf7/0x220 [usbcore]
[45219.232487]  [<ffffffffa008e411>] hub_thread+0x1321/0x21e0 [usbcore]
[45219.232535]  [<ffffffff810b3780>] ? wake_up_bit+0x50/0x50
[45219.232581]  [<ffffffffa008d0f0>] ? usb_remote_wakeup+0xb0/0xb0 [usbcore]
[45219.232628]  [<ffffffff810b2b9f>] kthread+0xdf/0xf0
[45219.232664]  [<ffffffff819a5574>] kernel_thread_helper+0x4/0x10
[45219.232706]  [<ffffffff819970b0>] ? retint_restore_args+0x13/0x13
[45219.232749]  [<ffffffff810b2ac0>] ? __init_kthread_worker+0x80/0x80
[45219.232791]  [<ffffffff819a5570>] ? gs_change+0x13/0x13
[45219.232826] Code: ff ff e9 d4 fb ff ff 0f 1f 80 00 00 00 00 55 48
89 e5 48 83 ec 10 66 66 66 66 90 48 83 05 2b aa 02 00 01 48 85 ff 48
89 f8 74 19 <8b> 17 48 83 05 21 aa 02 00 01 85 d2 74 0d f0 ff 00 48 83
05 2a
[45219.233184] RIP  [<ffffffffa0094e7d>] usb_get_urb+0x1d/0x70 [usbcore]
[45219.233240]  RSP <ffff8800554df8e0>
[45219.233268] ---[ end trace a7919e7f17c0a727 ]---
[45222.870031] usb0: no IPv6 routers present

>
>>
>> Following is a draft patch to guarantee the queue consistent, and
>> refer to expected skb in each loop cycle:
>>
>> diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
>> index b7b3f5b..6da0141 100644
>> --- a/drivers/net/usb/usbnet.c
>> +++ b/drivers/net/usb/usbnet.c
>> @@ -578,16 +578,28 @@ EXPORT_SYMBOL_GPL(usbnet_purge_paused_rxq);
>>  static int unlink_urbs (struct usbnet *dev, struct sk_buff_head *q)
>>  {
>>        unsigned long           flags;
>> -       struct sk_buff          *skb, *skbnext;
>> +       struct sk_buff          *skb;
>>        int                     count = 0;
>>
>>        spin_lock_irqsave (&q->lock, flags);
>> -       skb_queue_walk_safe(q, skb, skbnext) {
>> +       while (!skb_queue_empty(q)) {
>>                struct skb_data         *entry;
>>                struct urb              *urb;
>>                int                     retval;
>>
>> -               entry = (struct skb_data *) skb->cb;
>> +               skb_queue_walk(q, skb) {
>> +                       entry = (struct skb_data *)skb->cb;
>> +                       if (entry->state == rx_done ||
>> +                               entry->state == tx_done ||
>> +                               entry->state == rx_cleanup)
>
> Maybe it is not necessary, if the state has been changed to  rx_done
> or tx_done or rx_cleanup, it means the URB referenced to by the skb
> has been completed, and the coming usb_unlink_urb can handle the
> case correctly.
>

If its state is x_done/tx_done/rx_cleanup, that means the the skb will
be released soon, right? If so, it should avoid calling
usb_unlink_urb().

>> +                               continue;
>> +                       else
>> +                               break;
>> +               }
>> +
>> +               if (skb == (struct sk_buff *)(q))
>> +                       break;
>> +
>>                urb = entry->urb;
>>
>>
>> Thanks,
>> --Huajun Li
>
>
> Thanks,
> --
> Ming Lei
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [patch net-next 3/3] team: add per-port option for enabling/disabling ports
From: Jiri Pirko @ 2012-04-20 14:42 UTC (permalink / raw)
  To: netdev; +Cc: davem, eric.dumazet
In-Reply-To: <1334932926-23996-1-git-send-email-jpirko@redhat.com>

Signed-off-by: Jiri Pirko <jpirko@redhat.com>
---
 drivers/net/team/team.c |   24 ++++++++++++++++++++++++
 1 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
index fe7ca40..c61ae35 100644
--- a/drivers/net/team/team.c
+++ b/drivers/net/team/team.c
@@ -904,6 +904,23 @@ static int team_mode_option_set(struct team *team, struct team_gsetter_ctx *ctx)
 	return team_change_mode(team, ctx->data.str_val);
 }
 
+static int team_port_en_option_get(struct team *team,
+				   struct team_gsetter_ctx *ctx)
+{
+	ctx->data.bool_val = team_port_enabled(ctx->port);
+	return 0;
+}
+
+static int team_port_en_option_set(struct team *team,
+				   struct team_gsetter_ctx *ctx)
+{
+	if (ctx->data.bool_val)
+		team_port_enable(team, ctx->port);
+	else
+		team_port_disable(team, ctx->port);
+	return 0;
+}
+
 static int team_user_linkup_option_get(struct team *team,
 				       struct team_gsetter_ctx *ctx)
 {
@@ -946,6 +963,13 @@ static const struct team_option team_options[] = {
 		.setter = team_mode_option_set,
 	},
 	{
+		.name = "enabled",
+		.type = TEAM_OPTION_TYPE_BOOL,
+		.per_port = true,
+		.getter = team_port_en_option_get,
+		.setter = team_port_en_option_set,
+	},
+	{
 		.name = "user_linkup",
 		.type = TEAM_OPTION_TYPE_BOOL,
 		.per_port = true,
-- 
1.7.9.1

^ permalink raw reply related

* [patch net-next 2/3] team: allow to enable/disable ports
From: Jiri Pirko @ 2012-04-20 14:42 UTC (permalink / raw)
  To: netdev; +Cc: davem, eric.dumazet
In-Reply-To: <1334932926-23996-1-git-send-email-jpirko@redhat.com>

This patch changes content of hashlist (used to get port struct by
computed index (0...en_port_count-1)). Now the hash list contains only
enabled ports so userspace will be able to say what ports can be used
for tx/rx. This becomes handy when userspace will need to disable ports
which does not belong to active aggregator. By default, newly added port
is enabled.

Signed-off-by: Jiri Pirko <jpirko@redhat.com>
---
 drivers/net/team/team.c                  |   51 ++++++++++++++++++++---------
 drivers/net/team/team_mode_loadbalance.c |    2 +-
 drivers/net/team/team_mode_roundrobin.c  |    2 +-
 include/linux/if_team.h                  |   15 +++++----
 4 files changed, 45 insertions(+), 25 deletions(-)

diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
index 153a62d..fe7ca40 100644
--- a/drivers/net/team/team.c
+++ b/drivers/net/team/team.c
@@ -559,6 +559,8 @@ static int team_change_mode(struct team *team, const char *kind)
  * Rx path frame handler
  ************************/
 
+static bool team_port_enabled(struct team_port *port);
+
 /* note: already called with rcu_read_lock */
 static rx_handler_result_t team_handle_frame(struct sk_buff **pskb)
 {
@@ -575,8 +577,12 @@ static rx_handler_result_t team_handle_frame(struct sk_buff **pskb)
 
 	port = team_port_get_rcu(skb->dev);
 	team = port->team;
-
-	res = team->ops.receive(team, port, skb);
+	if (!team_port_enabled(port)) {
+		/* allow exact match delivery for disabled ports */
+		res = RX_HANDLER_EXACT;
+	} else {
+		res = team->ops.receive(team, port, skb);
+	}
 	if (res == RX_HANDLER_ANOTHER) {
 		struct team_pcpu_stats *pcpu_stats;
 
@@ -612,17 +618,25 @@ static bool team_port_find(const struct team *team,
 	return false;
 }
 
+static bool team_port_enabled(struct team_port *port)
+{
+	return port->index != -1;
+}
+
 /*
- * Add/delete port to the team port list. Write guarded by rtnl_lock.
- * Takes care of correct port->index setup (might be racy).
+ * Enable/disable port by adding to enabled port hashlist and setting
+ * port->index (Might be racy so reader could see incorrect ifindex when
+ * processing a flying packet, but that is not a problem). Write guarded
+ * by team->lock.
  */
-static void team_port_list_add_port(struct team *team,
-				    struct team_port *port)
+static void team_port_enable(struct team *team,
+			     struct team_port *port)
 {
-	port->index = team->port_count++;
+	if (team_port_enabled(port))
+		return;
+	port->index = team->en_port_count++;
 	hlist_add_head_rcu(&port->hlist,
 			   team_port_index_hash(team, port->index));
-	list_add_tail_rcu(&port->list, &team->port_list);
 }
 
 static void __reconstruct_port_hlist(struct team *team, int rm_index)
@@ -630,7 +644,7 @@ static void __reconstruct_port_hlist(struct team *team, int rm_index)
 	int i;
 	struct team_port *port;
 
-	for (i = rm_index + 1; i < team->port_count; i++) {
+	for (i = rm_index + 1; i < team->en_port_count; i++) {
 		port = team_get_port_by_index(team, i);
 		hlist_del_rcu(&port->hlist);
 		port->index--;
@@ -639,15 +653,17 @@ static void __reconstruct_port_hlist(struct team *team, int rm_index)
 	}
 }
 
-static void team_port_list_del_port(struct team *team,
-				   struct team_port *port)
+static void team_port_disable(struct team *team,
+			      struct team_port *port)
 {
 	int rm_index = port->index;
 
+	if (!team_port_enabled(port))
+		return;
 	hlist_del_rcu(&port->hlist);
-	list_del_rcu(&port->list);
 	__reconstruct_port_hlist(team, rm_index);
-	team->port_count--;
+	team->en_port_count--;
+	port->index = -1;
 }
 
 #define TEAM_VLAN_FEATURES (NETIF_F_ALL_CSUM | NETIF_F_SG | \
@@ -800,7 +816,9 @@ static int team_port_add(struct team *team, struct net_device *port_dev)
 		goto err_option_port_add;
 	}
 
-	team_port_list_add_port(team, port);
+	port->index = -1;
+	team_port_enable(team, port);
+	list_add_tail_rcu(&port->list, &team->port_list);
 	team_adjust_ops(team);
 	__team_compute_features(team);
 	__team_port_change_check(port, !!netif_carrier_ok(port_dev));
@@ -849,7 +867,8 @@ static int team_port_del(struct team *team, struct net_device *port_dev)
 
 	port->removed = true;
 	__team_port_change_check(port, false);
-	team_port_list_del_port(team, port);
+	team_port_disable(team, port);
+	list_del_rcu(&port->list);
 	team_adjust_ops(team);
 	team_option_port_del(team, port);
 	netdev_rx_handler_unregister(port_dev);
@@ -956,7 +975,7 @@ static int team_init(struct net_device *dev)
 		return -ENOMEM;
 
 	for (i = 0; i < TEAM_PORT_HASHENTRIES; i++)
-		INIT_HLIST_HEAD(&team->port_hlist[i]);
+		INIT_HLIST_HEAD(&team->en_port_hlist[i]);
 	INIT_LIST_HEAD(&team->port_list);
 
 	team_adjust_ops(team);
diff --git a/drivers/net/team/team_mode_loadbalance.c b/drivers/net/team/team_mode_loadbalance.c
index 438d5b8..86e8183 100644
--- a/drivers/net/team/team_mode_loadbalance.c
+++ b/drivers/net/team/team_mode_loadbalance.c
@@ -38,7 +38,7 @@ static bool lb_transmit(struct team *team, struct sk_buff *skb)
 	if (unlikely(!fp))
 		goto drop;
 	hash = SK_RUN_FILTER(fp, skb);
-	port_index = hash % team->port_count;
+	port_index = hash % team->en_port_count;
 	port = team_get_port_by_index_rcu(team, port_index);
 	if (unlikely(!port))
 		goto drop;
diff --git a/drivers/net/team/team_mode_roundrobin.c b/drivers/net/team/team_mode_roundrobin.c
index a0e8f80..6abfbdc 100644
--- a/drivers/net/team/team_mode_roundrobin.c
+++ b/drivers/net/team/team_mode_roundrobin.c
@@ -50,7 +50,7 @@ static bool rr_transmit(struct team *team, struct sk_buff *skb)
 	struct team_port *port;
 	int port_index;
 
-	port_index = rr_priv(team)->sent_packets++ % team->port_count;
+	port_index = rr_priv(team)->sent_packets++ % team->en_port_count;
 	port = team_get_port_by_index_rcu(team, port_index);
 	port = __get_first_port_up(team, port);
 	if (unlikely(!port))
diff --git a/include/linux/if_team.h b/include/linux/if_team.h
index 5fd5ab1..8185f57 100644
--- a/include/linux/if_team.h
+++ b/include/linux/if_team.h
@@ -28,10 +28,10 @@ struct team;
 
 struct team_port {
 	struct net_device *dev;
-	struct hlist_node hlist; /* node in hash list */
+	struct hlist_node hlist; /* node in enabled ports hash list */
 	struct list_head list; /* node in ordinary list */
 	struct team *team;
-	int index;
+	int index; /* index of enabled port. If disabled, it's set to -1 */
 
 	bool linkup; /* either state.linkup or user.linkup */
 
@@ -125,11 +125,12 @@ struct team {
 	struct mutex lock; /* used for overall locking, e.g. port lists write */
 
 	/*
-	 * port lists with port count
+	 * List of enabled ports and their count
 	 */
-	int port_count;
-	struct hlist_head port_hlist[TEAM_PORT_HASHENTRIES];
-	struct list_head port_list;
+	int en_port_count;
+	struct hlist_head en_port_hlist[TEAM_PORT_HASHENTRIES];
+
+	struct list_head port_list; /* list of all ports */
 
 	struct list_head option_list;
 	struct list_head option_inst_list; /* list of option instances */
@@ -142,7 +143,7 @@ struct team {
 static inline struct hlist_head *team_port_index_hash(struct team *team,
 						      int port_index)
 {
-	return &team->port_hlist[port_index & (TEAM_PORT_HASHENTRIES - 1)];
+	return &team->en_port_hlist[port_index & (TEAM_PORT_HASHENTRIES - 1)];
 }
 
 static inline struct team_port *team_get_port_by_index(struct team *team,
-- 
1.7.9.1

^ permalink raw reply related

* [patch net-next 1/3] team: lb: let userspace care about port macs
From: Jiri Pirko @ 2012-04-20 14:42 UTC (permalink / raw)
  To: netdev; +Cc: davem, eric.dumazet
In-Reply-To: <1334932926-23996-1-git-send-email-jpirko@redhat.com>

Better to leave this for userspace

Signed-off-by: Jiri Pirko <jpirko@redhat.com>
---
 drivers/net/team/team_mode_loadbalance.c |   12 ------------
 1 files changed, 0 insertions(+), 12 deletions(-)

diff --git a/drivers/net/team/team_mode_loadbalance.c b/drivers/net/team/team_mode_loadbalance.c
index 2b506b2..438d5b8 100644
--- a/drivers/net/team/team_mode_loadbalance.c
+++ b/drivers/net/team/team_mode_loadbalance.c
@@ -142,22 +142,10 @@ static void lb_exit(struct team *team)
 				ARRAY_SIZE(lb_options));
 }
 
-static int lb_port_enter(struct team *team, struct team_port *port)
-{
-	return team_port_set_team_mac(port);
-}
-
-static void lb_port_change_mac(struct team *team, struct team_port *port)
-{
-	team_port_set_team_mac(port);
-}
-
 static const struct team_mode_ops lb_mode_ops = {
 	.init			= lb_init,
 	.exit			= lb_exit,
 	.transmit		= lb_transmit,
-	.port_enter		= lb_port_enter,
-	.port_change_mac	= lb_port_change_mac,
 };
 
 static struct team_mode lb_mode = {
-- 
1.7.9.1

^ permalink raw reply related

* [patch net-next 0/3] team: couple patches
From: Jiri Pirko @ 2012-04-20 14:42 UTC (permalink / raw)
  To: netdev; +Cc: davem, eric.dumazet

Jiri Pirko (3):
  team: lb: let userspace care about port macs
  team: allow to enable/disable ports
  team: add per-port option for enabling/disabling ports

 drivers/net/team/team.c                  |   75 +++++++++++++++++++++++------
 drivers/net/team/team_mode_loadbalance.c |   14 +-----
 drivers/net/team/team_mode_roundrobin.c  |    2 +-
 include/linux/if_team.h                  |   15 +++---
 4 files changed, 69 insertions(+), 37 deletions(-)

-- 
1.7.9.1

^ permalink raw reply

* Re: [PATCH net-next 04/19] net: Kill register_sysctl_rotable
From: Eric W. Biederman @ 2012-04-20 14:42 UTC (permalink / raw)
  To: Serge E. Hallyn
  Cc: David Miller, netdev, Gao feng, pablo, Stephen Hemminger,
	Pavel Emelyanov
In-Reply-To: <20120420135323.GA4877@mail.hallyn.com>

"Serge E. Hallyn" <serge@hallyn.com> writes:

> Quoting Eric W. Biederman (ebiederm@xmission.com):
>> 
>> register_sysctl_rotable never caught on as an interesting way to
>> register sysctls.  My take on the situation is that what we want are
>> sysctls that we can only see in the initial network namespace.  What we
>> have implemented with register_sysctl_rotable are sysctls that we can
>> see in all of the network namespaces and can only change in the initial
>> network namespace.
>> 
>> That is a very silly way to go.  Just register the network sysctls
>> in the initial network namespace and we don't have any weird special
>> cases to deal with.
>> 
>> The sysctls affected are:
>> /proc/sys/net/ipv4/ipfrag_secret_interval
>> /proc/sys/net/ipv4/ipfrag_max_dist
>> /proc/sys/net/ipv6/ip6frag_secret_interval
>> /proc/sys/net/ipv6/mld_max_msf
>> 
>> I really don't expect anyone will miss them if they can't read them in a
>> child user namespace.
>
> If there was something userspace could do to work around certain values
> of these settings then I'd say keeping the readonly values is worthwhile,
> but AFAICS if a bad network context requires ipfrag_max_dist 0, there's
> nothing userspace can do about it...
>
>
> So from a container pov view at least, I'm happy with this.  I'm far from
> qualified on the netns code itself, but taking a look in the unlikely case
> I can spot something :)

In this case I figured I would copy you and a few others who have been
talking about similar things recently, and also because you might care
that a whole bunch of networking sysctls that aren't per network
namespace will stop showing up in containers.

It is my hope that we use some of these same mechanisms that allow per
network namespace sysctls will be used to allow per pid and uts
namespace sysctls as well.  It isn't as important as the files don't
change, but we can do it cleanly and one of these days I will get around
to making /proc/sys a symlink to /proc/<pid>/sys so that I can remove
the very unorthodox d_compare tricks that we use today.

The sysctl internal data structures are now a hair cleaner than what
sysfs uses for the same class of problem so I might someday go back and
fix sysfs to use the same idea of internal links, so I can get the sysfs
dirent size down some more, and be able to more cleanly isolate the
namespace handling from the rest of the sysfs code.  It isn't bad today
but it is the source of most of the surprises and bugs when people tweak
the sysfs code.

Anyway I ramble.  Now I need to get back to your review comments on my
user namespace patchset.

Thanks for taking a glance here,
Eric

^ permalink raw reply

* Re: use-after-free in usbnet
From: Ming Lei @ 2012-04-20 14:22 UTC (permalink / raw)
  To: Huajun Li
  Cc: Oliver Neukum, Alan Stern, Dave Jones, netdev, linux-usb,
	Fedora Kernel Team
In-Reply-To: <CA+v9cxawVwKakF6c_RpAw2XUGWcbqd8M+ZJqyq76Au9rmNosmQ@mail.gmail.com>

On Fri, Apr 20, 2012 at 9:37 PM, Huajun Li <huajun.li.lee@gmail.com> wrote:
>
> Above patch has already been integrated to mainline. However, maybe
> there still exists another potentail use-after-free issue, here is a
> case:
>      After release the lock in unlink_urbs(), defer_bh() may move
> current skb from rxq/txq to dev->done queue, even cause the skb be
> released. Then in next loop cycle, it can't refer to expected skb, and
> may Oops again.

Could you explain in a bit detail? Why can't the expected skb be refered
to in next loop?

>
> To easily reproduce it, in unlink_urbs(), you can delay a short time
> after usb_put_urb(urb), then disconnect your device while transferring
> data, and repeat it times you will find errors on your screen.

Could you post out the error log?

>
> Following is a draft patch to guarantee the queue consistent, and
> refer to expected skb in each loop cycle:
>
> diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
> index b7b3f5b..6da0141 100644
> --- a/drivers/net/usb/usbnet.c
> +++ b/drivers/net/usb/usbnet.c
> @@ -578,16 +578,28 @@ EXPORT_SYMBOL_GPL(usbnet_purge_paused_rxq);
>  static int unlink_urbs (struct usbnet *dev, struct sk_buff_head *q)
>  {
>        unsigned long           flags;
> -       struct sk_buff          *skb, *skbnext;
> +       struct sk_buff          *skb;
>        int                     count = 0;
>
>        spin_lock_irqsave (&q->lock, flags);
> -       skb_queue_walk_safe(q, skb, skbnext) {
> +       while (!skb_queue_empty(q)) {
>                struct skb_data         *entry;
>                struct urb              *urb;
>                int                     retval;
>
> -               entry = (struct skb_data *) skb->cb;
> +               skb_queue_walk(q, skb) {
> +                       entry = (struct skb_data *)skb->cb;
> +                       if (entry->state == rx_done ||
> +                               entry->state == tx_done ||
> +                               entry->state == rx_cleanup)

Maybe it is not necessary, if the state has been changed to  rx_done
or tx_done or rx_cleanup, it means the URB referenced to by the skb
has been completed, and the coming usb_unlink_urb can handle the
case correctly.

> +                               continue;
> +                       else
> +                               break;
> +               }
> +
> +               if (skb == (struct sk_buff *)(q))
> +                       break;
> +
>                urb = entry->urb;
>
>
> Thanks,
> --Huajun Li


Thanks,
-- 
Ming Lei

^ permalink raw reply

* Re: RTM_NEWLINK not received by application when connecting multiple devices simultaneously
From: Kristian Evensen @ 2012-04-20 14:07 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Ben Greear, netdev
In-Reply-To: <20120419133635.7d816d67@nehalam.linuxnetplumber.net>

Thank you very much for all the help.

>> I spent some more time debugging this now. It turns out that which
>> interface is seen by my application is not random, it is always the
>> first one that is connected. This indicates that the bug is that the
>> netlink message contains information about more than one interface.
>> However, I am not able to prove this.
>
> There is no filtering. A dump request always returns all interfaces.

I think maybe I didnt explain myself clearly enough, I am sorry. This
does not happen when I dump interface info, but at the event of
connecting two interfaces at the same time.

I have now converted the application to using libmnl, but I still see
the same error. There is one message that is not received by my
netlink socket, or at least, the application does not process it. Just
in case it was bundled together with the message about the first
interface, I tried adding a loop where I received the packets (similar
to the one in the -dump example). However, this did not help.

-Kristian

^ permalink raw reply

* Re: [PATCH net-next 04/19] net: Kill register_sysctl_rotable
From: Serge E. Hallyn @ 2012-04-20 13:53 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: David Miller, netdev, Serge E. Hallyn, Gao feng, pablo,
	Stephen Hemminger, Pavel Emelyanov
In-Reply-To: <m1obqngvcg.fsf@fess.ebiederm.org>

Quoting Eric W. Biederman (ebiederm@xmission.com):
> 
> register_sysctl_rotable never caught on as an interesting way to
> register sysctls.  My take on the situation is that what we want are
> sysctls that we can only see in the initial network namespace.  What we
> have implemented with register_sysctl_rotable are sysctls that we can
> see in all of the network namespaces and can only change in the initial
> network namespace.
> 
> That is a very silly way to go.  Just register the network sysctls
> in the initial network namespace and we don't have any weird special
> cases to deal with.
> 
> The sysctls affected are:
> /proc/sys/net/ipv4/ipfrag_secret_interval
> /proc/sys/net/ipv4/ipfrag_max_dist
> /proc/sys/net/ipv6/ip6frag_secret_interval
> /proc/sys/net/ipv6/mld_max_msf
> 
> I really don't expect anyone will miss them if they can't read them in a
> child user namespace.

If there was something userspace could do to work around certain values
of these settings then I'd say keeping the readonly values is worthwhile,
but AFAICS if a bad network context requires ipfrag_max_dist 0, there's
nothing userspace can do about it...

So from a container pov view at least, I'm happy with this.  I'm far from
qualified on the netns code itself, but taking a look in the unlikely case
I can spot something :)

> CC: Pavel Emelyanov <xemul@openvz.org>
> Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
> ---
>  include/net/net_namespace.h |    2 --
>  net/core/sysctl_net_core.c  |    2 +-
>  net/ipv4/ip_fragment.c      |    2 +-
>  net/ipv6/reassembly.c       |    2 +-
>  net/ipv6/sysctl_net_ipv6.c  |    2 +-
>  net/sysctl_net.c            |   23 -----------------------
>  6 files changed, 4 insertions(+), 29 deletions(-)
> 
> diff --git a/include/net/net_namespace.h b/include/net/net_namespace.h
> index 767dcd40..6413fcb 100644
> --- a/include/net/net_namespace.h
> +++ b/include/net/net_namespace.h
> @@ -290,8 +290,6 @@ static inline int net_sysctl_init(void) { return 0; }
>  #endif
>  extern struct ctl_table_header *register_net_sysctl_table(struct net *net,
>  	const struct ctl_path *path, struct ctl_table *table);
> -extern struct ctl_table_header *register_net_sysctl_rotable(
> -	const struct ctl_path *path, struct ctl_table *table);
>  extern struct ctl_table_header *register_net_sysctl(struct net *net,
>  	const char *path, struct ctl_table *table);
>  extern void unregister_net_sysctl_table(struct ctl_table_header *header);
> diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
> index cee5991..9504086 100644
> --- a/net/core/sysctl_net_core.c
> +++ b/net/core/sysctl_net_core.c
> @@ -257,7 +257,7 @@ static __init int sysctl_core_init(void)
>  	static struct ctl_table empty[1];
>  
>  	kmemleak_not_leak(register_sysctl_paths(net_core_path, empty));
> -	register_net_sysctl_rotable(net_core_path, net_core_table);
> +	register_net_sysctl(&init_net, "net/core", net_core_table);
>  	return register_pernet_subsys(&sysctl_core_ops);
>  }
>  
> diff --git a/net/ipv4/ip_fragment.c b/net/ipv4/ip_fragment.c
> index 3727e23..a746cca 100644
> --- a/net/ipv4/ip_fragment.c
> +++ b/net/ipv4/ip_fragment.c
> @@ -807,7 +807,7 @@ static void __net_exit ip4_frags_ns_ctl_unregister(struct net *net)
>  
>  static void ip4_frags_ctl_register(void)
>  {
> -	register_net_sysctl_rotable(net_ipv4_ctl_path, ip4_frags_ctl_table);
> +	register_net_sysctl(&init_net, "net/ipv4", ip4_frags_ctl_table);
>  }
>  #else
>  static inline int ip4_frags_ns_ctl_register(struct net *net)
> diff --git a/net/ipv6/reassembly.c b/net/ipv6/reassembly.c
> index 9447bd6..42f4f7c 100644
> --- a/net/ipv6/reassembly.c
> +++ b/net/ipv6/reassembly.c
> @@ -674,7 +674,7 @@ static struct ctl_table_header *ip6_ctl_header;
>  
>  static int ip6_frags_sysctl_register(void)
>  {
> -	ip6_ctl_header = register_net_sysctl_rotable(net_ipv6_ctl_path,
> +	ip6_ctl_header = register_net_sysctl(&init_net, "net/ipv6",
>  			ip6_frags_ctl_table);
>  	return ip6_ctl_header == NULL ? -ENOMEM : 0;
>  }
> diff --git a/net/ipv6/sysctl_net_ipv6.c b/net/ipv6/sysctl_net_ipv6.c
> index 166a57c..06f21e5 100644
> --- a/net/ipv6/sysctl_net_ipv6.c
> +++ b/net/ipv6/sysctl_net_ipv6.c
> @@ -140,7 +140,7 @@ int ipv6_sysctl_register(void)
>  {
>  	int err = -ENOMEM;
>  
> -	ip6_header = register_net_sysctl_rotable(net_ipv6_ctl_path, ipv6_rotable);
> +	ip6_header = register_net_sysctl(&init_net, "net/ipv6", ipv6_rotable);
>  	if (ip6_header == NULL)
>  		goto out;
>  
> diff --git a/net/sysctl_net.c b/net/sysctl_net.c
> index ce97237..2b8d1d9 100644
> --- a/net/sysctl_net.c
> +++ b/net/sysctl_net.c
> @@ -59,19 +59,6 @@ static struct ctl_table_root net_sysctl_root = {
>  	.permissions = net_ctl_permissions,
>  };
>  
> -static int net_ctl_ro_header_perms(struct ctl_table_root *root,
> -		struct nsproxy *namespaces, struct ctl_table *table)
> -{
> -	if (net_eq(namespaces->net_ns, &init_net))
> -		return table->mode;
> -	else
> -		return table->mode & ~0222;
> -}
> -
> -static struct ctl_table_root net_sysctl_ro_root = {
> -	.permissions = net_ctl_ro_header_perms,
> -};
> -
>  static int __net_init sysctl_net_init(struct net *net)
>  {
>  	setup_sysctl_set(&net->sysctls, &net_sysctl_root, is_seen);
> @@ -103,8 +90,6 @@ __init int net_sysctl_init(void)
>  	ret = register_pernet_subsys(&sysctl_pernet_ops);
>  	if (ret)
>  		goto out;
> -	setup_sysctl_set(&net_sysctl_ro_root.default_set, &net_sysctl_ro_root, NULL);
> -	register_sysctl_root(&net_sysctl_ro_root);
>  	register_sysctl_root(&net_sysctl_root);
>  out:
>  	return ret;
> @@ -117,14 +102,6 @@ struct ctl_table_header *register_net_sysctl_table(struct net *net,
>  }
>  EXPORT_SYMBOL_GPL(register_net_sysctl_table);
>  
> -struct ctl_table_header *register_net_sysctl_rotable(const
> -		struct ctl_path *path, struct ctl_table *table)
> -{
> -	return __register_sysctl_paths(&net_sysctl_ro_root.default_set,
> -					path, table);
> -}
> -EXPORT_SYMBOL_GPL(register_net_sysctl_rotable);
> -
>  struct ctl_table_header *register_net_sysctl(struct net *net,
>  	const char *path, struct ctl_table *table)
>  {
> -- 
> 1.7.2.5

^ permalink raw reply

* Re: use-after-free in usbnet
From: Huajun Li @ 2012-04-20 13:37 UTC (permalink / raw)
  To: Ming Lei, Oliver Neukum, Alan Stern, Dave Jones
  Cc: netdev, linux-usb, Fedora Kernel Team, Huajun Li
In-Reply-To: <CACVXFVODFvtF1DRyWJmnYCByw7sr9QDv_+Taxo5maVaUauDNtg@mail.gmail.com>

On Thu, Mar 22, 2012 at 5:30 PM, Ming Lei <tom.leiming@gmail.com> wrote:
> On Thu, Mar 22, 2012 at 5:08 PM, Oliver Neukum <oneukum@suse.de> wrote:
>>
>> this looks good, but could you add a comment explaining the reason for
>> taking a reference?
>
> OK, I will post a formal one if you have no objection on the below.
>
> diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
> index 4b8b52c..febfdce 100644
> --- a/drivers/net/usb/usbnet.c
> +++ b/drivers/net/usb/usbnet.c
> @@ -589,6 +589,14 @@ static int unlink_urbs (struct usbnet *dev,
> struct sk_buff_head *q)
>                entry = (struct skb_data *) skb->cb;
>                urb = entry->urb;
>
> +               /*
> +                * Get a reference count of the URB to avoid it to be
> +                * freed during usb_unlink_urb, which may trigger
> +                * use-after-free problem inside usb_unlink_urb since
> +                * usb_unlink_urb is always racing with .complete
> +                * handler(include defer_bh).
> +                */
> +               usb_get_urb(urb);
>                spin_unlock_irqrestore(&q->lock, flags);
>                // during some PM-driven resume scenarios,
>                // these (async) unlinks complete immediately
> @@ -597,6 +605,7 @@ static int unlink_urbs (struct usbnet *dev, struct
> sk_buff_head *q)
>                        netdev_dbg(dev->net, "unlink urb err, %d\n", retval);
>                else
>                        count++;
> +               usb_put_urb(urb);
>                spin_lock_irqsave(&q->lock, flags);
>        }
>        spin_unlock_irqrestore (&q->lock, flags);
>
>


Above patch has already been integrated to mainline. However, maybe
there still exists another potentail use-after-free issue, here is a
case:
      After release the lock in unlink_urbs(), defer_bh() may move
current skb from rxq/txq to dev->done queue, even cause the skb be
released. Then in next loop cycle, it can't refer to expected skb, and
may Oops again.

To easily reproduce it, in unlink_urbs(), you can delay a short time
after usb_put_urb(urb), then disconnect your device while transferring
data, and repeat it times you will find errors on your screen.

Following is a draft patch to guarantee the queue consistent, and
refer to expected skb in each loop cycle:

diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index b7b3f5b..6da0141 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -578,16 +578,28 @@ EXPORT_SYMBOL_GPL(usbnet_purge_paused_rxq);
 static int unlink_urbs (struct usbnet *dev, struct sk_buff_head *q)
 {
 	unsigned long		flags;
-	struct sk_buff		*skb, *skbnext;
+	struct sk_buff		*skb;
 	int			count = 0;

 	spin_lock_irqsave (&q->lock, flags);
-	skb_queue_walk_safe(q, skb, skbnext) {
+	while (!skb_queue_empty(q)) {
 		struct skb_data		*entry;
 		struct urb		*urb;
 		int			retval;

-		entry = (struct skb_data *) skb->cb;
+		skb_queue_walk(q, skb) {
+			entry = (struct skb_data *)skb->cb;
+			if (entry->state == rx_done ||
+				entry->state == tx_done ||
+				entry->state == rx_cleanup)
+				continue;
+			else
+				break;
+		}
+
+		if (skb == (struct sk_buff *)(q))
+			break;
+
 		urb = entry->urb;


Thanks,
--Huajun Li

^ permalink raw reply related

* Re: fake rtable dst patch applied but kernel keeps panicing
From: Eric Dumazet @ 2012-04-20 13:22 UTC (permalink / raw)
  To: Massimo Cetra; +Cc: netdev, peter.huangpeng
In-Reply-To: <4F913429.2040705@navynet.it>

On Fri, 2012-04-20 at 12:02 +0200, Massimo Cetra wrote:

> 
> i applied this patch and Peters last patch to a 3.3.2 kernel.
> The result was a bit disappointing because the step was backwards.
> 
> Locally, from the same machine, i could ping each IP of each tun 
> interface used by any virtual server.
> 
>  From the LAN such addresses were not pingable while the ip address of 
> the bridge was reachable.
> 
> Max

I dont know, this code is crap and should be fixed.

This nf_bridge_unshare() is obviously buggy as hell.

^ permalink raw reply

* Re: [PATCH] drop_monitor: allow more events per second
From: Neil Horman @ 2012-04-20 13:06 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, netdev
In-Reply-To: <1334870733.4810.0.camel@edumazet-laptop>

On Thu, Apr 19, 2012 at 11:25:33PM +0200, Eric Dumazet wrote:
> Le jeudi 19 avril 2012 à 13:28 -0400, Neil Horman a écrit :
> 
> > I spent a good deal of time going through it to make sure it was preempt safe,
> > but its certainly possible that I missed something.  I'll look at it shortly.
> > Thanks for this update though
> > 
> 
> You might want to check this problem as well :
> 
> [   38.352571] BUG: sleeping function called from invalid context at kernel/mutex.c:85
> [   38.352576] in_atomic(): 1, irqs_disabled(): 0, pid: 4415, name: dropwatch
> [   38.352580] Pid: 4415, comm: dropwatch Not tainted 3.4.0-rc2+ #71
> [   38.352582] Call Trace:
> [   38.352592]  [<ffffffff8153aaf0>] ? trace_napi_poll_hit+0xd0/0xd0
> [   38.352599]  [<ffffffff81063f2a>] __might_sleep+0xca/0xf0
> [   38.352606]  [<ffffffff81655b16>] mutex_lock+0x26/0x50
> [   38.352610]  [<ffffffff8153aaf0>] ? trace_napi_poll_hit+0xd0/0xd0
> [   38.352616]  [<ffffffff810b72d9>] tracepoint_probe_register+0x29/0x90
> [   38.352621]  [<ffffffff8153a585>] set_all_monitor_traces+0x105/0x170
> [   38.352625]  [<ffffffff8153a8ca>] net_dm_cmd_trace+0x2a/0x40
> [   38.352630]  [<ffffffff8154a81a>] genl_rcv_msg+0x21a/0x2b0
> [   38.352636]  [<ffffffff810f8029>] ? zone_statistics+0x99/0xc0
> [   38.352640]  [<ffffffff8154a600>] ? genl_rcv+0x30/0x30
> [   38.352645]  [<ffffffff8154a059>] netlink_rcv_skb+0xa9/0xd0
> [   38.352649]  [<ffffffff8154a5f0>] genl_rcv+0x20/0x30
> [   38.352653]  [<ffffffff81549a7e>] netlink_unicast+0x1ae/0x1f0
> [   38.352658]  [<ffffffff81549d76>] netlink_sendmsg+0x2b6/0x310
> [   38.352663]  [<ffffffff8150824f>] sock_sendmsg+0x10f/0x130
> [   38.352668]  [<ffffffff8150abe0>] ? move_addr_to_kernel+0x60/0xb0
> [   38.352673]  [<ffffffff81515f04>] ? verify_iovec+0x64/0xe0
> [   38.352677]  [<ffffffff81509c46>] __sys_sendmsg+0x386/0x390
> [   38.352682]  [<ffffffff810ffaf9>] ? handle_mm_fault+0x139/0x210
> [   38.352687]  [<ffffffff8165b5bc>] ? do_page_fault+0x1ec/0x4f0
> [   38.352693]  [<ffffffff8106ba4d>] ? set_next_entity+0x9d/0xb0
> [   38.352699]  [<ffffffff81310b49>] ? tty_ldisc_deref+0x9/0x10
> [   38.352703]  [<ffffffff8106d363>] ? pick_next_task_fair+0x63/0x140
> [   38.352708]  [<ffffffff8150b8d4>] sys_sendmsg+0x44/0x80
> [   38.352713]  [<ffffffff8165f8e2>] system_call_fastpath+0x16/0x1b
> 
> 
> 

Will do, thanks!
Neil

^ permalink raw reply

* Question with secure_ipv4_port_ephemeral() implementation
From: Tetsuo Handa @ 2012-04-20 12:30 UTC (permalink / raw)
  To: netdev

Commit 6e5714ea "net: Compute protocol sequence numbers and fragment IDs using
MD5." removed periodic get_random_bytes() calls. After that commit,

  static u32 net_secret[MD5_MESSAGE_BYTES / 4] ____cacheline_aligned;

is filled with random bytes for only once upon boot and is never updated again.
Then, shouldn't net_secret be marked as __read_mostly?

Just from curiosity... what was the reason for changing
secure_ipv4_port_ephemeral() generate same return value for same arguments?
Was periodically changing return value for same arguments unfriendly with NAT
or something?

^ permalink raw reply

* Re: [PATCH v2 net-next] tcp: avoid expensive pskb_expand_head() calls
From: Ilpo Järvinen @ 2012-04-20 12:27 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Neal Cardwell, David Miller, netdev, Tom Herbert,
	Maciej Żenczykowski, Yuchung Cheng
In-Reply-To: <1334839490.2395.160.camel@edumazet-glaptop>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2004 bytes --]

On Thu, 19 Apr 2012, Eric Dumazet wrote:

> On Thu, 2012-04-19 at 14:57 +0300, Ilpo Järvinen wrote:
> 
> > I'm not concerned of the output side, that seems to work because 
> > of the in tcp_retransmit_skb getting rid of the extra first.
> > 
> > The ACK input stuff is more interesting, e.g., this one in 
> > tcp_mark_head_lost:
> > 
> > 	err = tcp_fragment(sk, skb, (packets - oldcnt) * mss, mss);
> > 
> > It splits from TCP_SKB_CB(skb)->seq + (packets - oldcnt) * mss whereas
> > I think the desired point would be: TCP_SKB_CB(skb)->seq + offset_ack + 
> > (packets - oldcnt) * mss?
> > 
> > ...There is similar case in sacktag code too while it's aligning to mss 
> > boundaries in tcp_match_skb_to_sack.
> 
> Hmm yes, so maybe its safer to update TCP_SKB_CB(skb)->seq in
> tcp_tso_acked() (as well as offset_ack) and make needed adjustements in
> tcp_fragment() if we find offset_ack being not null.

I suppose that somewhat works, it helps here a lot that you work only with 
the head skb making lot of cases not possible... I once made something 
similar (before I came up the current shift/merge approach) and ended up 
to this:
  http://www.mail-archive.com/netdev@vger.kernel.org/msg56191.html

...But...

There's another can of worms still it seems.... At least tcp_skb_seglen 
that returns weird results when pcount becomes 1!

...Somewhat related to the pcount == 1 problem, I've long wondered if the 
zeroed gso_size with pcount == 1 is worth keeping in the first place?

However, I kind of liked the neatness in the original approach where ->seq 
does not lie. That would have kept most of stuff very localized because 
each skb is still fully valid and consistent with itself, whereas 
introducing lies adds lots of hidden traps (except for the pcount of 
course, but consistency for it has not been there for some years
already :-)). The tcp_match_skb_to_sack code seems to actually work 
exactly because of this consistency (if I now on the second/third read got 
it right).


-- 
 i.

^ permalink raw reply

* [PATCH 2/2] iproute2: add ip-l2tp man page
From: James Chapman @ 2012-04-20 11:29 UTC (permalink / raw)
  To: netdev
In-Reply-To: <1334921383-32109-1-git-send-email-jchapman@katalix.com>

Add a man page to cover the "ip l2tp" commands. Add a reference to it
in the main ip page.

Signed-off-by: James Chapman <jchapman@katalix.com>
---
 man/man8/ip-l2tp.8 |  350 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 man/man8/ip.8      |    3 +-
 2 files changed, 352 insertions(+), 1 deletions(-)
 create mode 100644 man/man8/ip-l2tp.8

diff --git a/man/man8/ip-l2tp.8 b/man/man8/ip-l2tp.8
new file mode 100644
index 0000000..c115e7c
--- /dev/null
+++ b/man/man8/ip-l2tp.8
@@ -0,0 +1,350 @@
+.TH IP\-L2TP 8 "19 Apr 2012" "iproute2" "Linux"
+.SH "NAME"
+ip-l2tp - L2TPv3 static unmanaged tunnel configuration
+.SH "SYNOPSIS"
+.sp
+.ad l
+.in +8
+.ti -8
+.B ip
+.RI "[ " OPTIONS " ]"
+.B l2tp
+.RI " { " COMMAND " | "
+.BR help " }"
+.sp
+.ti -8
+.BR "ip l2tp add tunnel"
+.br
+.B remote
+.RI "[ " ADDR " ]"
+.B local
+.RI "[ " ADDR " ]"
+.br
+.B tunnel_id
+.IR ID
+.B peer_tunnel_id
+.IR ID
+.br
+.RB "[ " encap " { " ip " | " udp " } ]"
+.br
+.RB "[ " udp_sport
+.IR PORT
+.RB " ] [ " udp_dport
+.IR PORT
+.RB " ]"
+.br
+.ti -8
+.BR "ip l2tp add session"
+.RB "[ " name
+.IR NAME
+.RB " ]"
+.br
+.B tunnel_id
+.IR ID
+.B session_id
+.IR ID
+.B peer_session_id
+.IR ID
+.br
+.RB "[ " cookie
+.IR HEXSTR
+.RB " ] [ " peer_cookie
+.IR HEXSTR
+.RB " ]"
+.br
+.RB "[ " offset
+.IR OFFSET
+.RB " ] [ " peer_offset
+.IR OFFSET
+.RB " ]"
+.br
+.ti -8
+.BR "ip l2tp del tunnel"
+.B tunnel_id
+.IR ID
+.br
+.ti -8
+.BR "ip l2tp del session"
+.B tunnel_id
+.IR ID
+.B session_id
+.IR ID
+.br
+.ti -8
+.BR "ip l2tp show tunnel"
+.B "[" tunnel_id
+.IR ID
+.B "]"
+.br
+.ti -8
+.BR "ip l2tp show session"
+.B "[" tunnel_id
+.IR ID
+.B "] [" session_id
+.IR ID
+.B "]"
+.br
+.ti -8
+.IR NAME " := "
+.IR STRING
+.ti -8
+.IR ADDR " := { " IP_ADDRESS " }"
+.ti -8
+.IR PORT " := { " NUMBER " }"
+.ti -8
+.IR ID " := { " NUMBER " }"
+.ti -8
+.ti -8
+.IR HEXSTR " := { 8 or 16 hex digits (4 / 8 bytes) }"
+.SH DESCRIPTION
+The
+.B ip l2tp
+commands are used to establish static, or so-called
+.I unmanaged
+L2TPv3 ethernet tunnels. For unmanaged tunnels, there is no L2TP
+control protocol so no userspace daemon is required - tunnels are
+manually created by issuing commands at a local system and at a remote
+peer.
+.PP
+L2TPv3 is suitable for Layer-2 tunnelling. Static tunnels are useful
+to establish network links across IP networks when the tunnels are
+fixed. L2TPv3 tunnels can carry data of more than one session. Each
+session is identified by a session_id and its parent tunnel's
+tunnel_id. A tunnel must be created before a session can be created in
+the tunnel.
+.PP
+When creating an L2TP tunnel, the IP address of the remote peer is
+specified, which can be either an IPv4 or IPv6 address. The local IP
+address to be used to reach the peer must also be specified. This is
+the address on which the local system will listen for and accept
+received L2TP data packets from the peer.
+.PP
+L2TPv3 defines two packet encapsulation formats: UDP or IP. UDP
+encapsulation is most common. IP encapsulation uses a dedicated IP
+protocol value to carry L2TP data without the overhead of UDP. Use IP
+encapsulation only when there are no NAT devices or firewalls in the
+network path.
+.PP
+When an L2TPv3 ethernet session is created, a virtual network
+interface is created for the session, which must then be configured
+and brought up, just like any other network interface. When data is
+passed through the interface, it is carried over the L2TP tunnel to
+the peer. By configuring the system's routing tables or adding the
+interface to a bridge, the L2TP interface is like a virtual wire
+(pseudowire) connected to the peer.
+.PP
+Establishing an unmanaged L2TPv3 ethernet pseudowire involves manually
+creating L2TP contexts on the local system and at the peer. Parameters
+used at each site must correspond or no data will be passed. No
+consistency checks are possible since there is no control protocol
+used to establish unmanaged L2TP tunnels. Once the virtual network
+interface of a given L2TP session is configured and enabled, data can
+be transmitted, even if the peer isn't yet configured. If the peer
+isn't configured, the L2TP data packets will be discarded by
+the peer.
+.PP
+To establish an unmanaged L2TP tunnel, use
+.B l2tp add tunnel
+and
+.B l2tp add session
+commands described in this document. Then configure and enable the
+tunnel's virtual network interface, as required.
+.PP
+Note that unmanaged tunnels carry only ethernet frames. If you need to
+carry PPP traffic (L2TPv2) or your peer doesn't support unmanaged
+L2TPv3 tunnels, you will need an L2TP server which implements the L2TP
+control protocol. The L2TP control protocol allows dynamic L2TP
+tunnels and sessions to be established and provides for detecting and
+acting upon network failures.
+.SS ip l2tp add tunnel - add a new tunnel
+.TP
+.BI name " NAME "
+sets the session network interface name. Default is l2tpethN.
+.TP
+.BI tunnel_id " ID"
+set the tunnel id, which is a 32-bit integer value. Uniquely
+identifies the tunnel. The value used must match the peer_tunnel_id
+value being used at the peer.
+.TP
+.BI peer_tunnel_id " ID"
+set the peer tunnel id, which is a 32-bit integer value assigned to
+the tunnel by the peer. The value used must match the tunnel_id value
+being used at the peer.
+.TP
+.BI remote " ADDR"
+set the IP address of the remote peer. May be specified as an IPv4
+address or an IPv6 address.
+.TP
+.BI local " ADDR"
+set the IP address of the local interface to be used for the
+tunnel. This address must be the address of a local interface. May be
+specified as an IPv4 address or an IPv6 address.
+.TP
+.BI encap " ENCAP"
+set the encapsulation type of the tunnel.
+.br
+Valid values for encapsulation are:
+.BR udp ", " ip "."
+.TP
+.BI udp_sport " PORT"
+set the UDP source port to be used for the tunnel. Must be present
+when udp encapsulation is selected. Ignored when ip encapsulation is
+selected.
+.TP
+.BI udp_dport " PORT"
+set the UDP destination port to be used for the tunnel. Must be
+present when udp encapsulation is selected. Ignored when ip
+encapsulation is selected.
+.SS ip l2tp del tunnel - destroy a tunnel
+.TP
+.BI tunnel_id " ID"
+set the tunnel id of the tunnel to be deleted. All sessions within the
+tunnel must be deleted first.
+.SS ip l2tp show tunnel - show information about tunnels
+.TP
+.BI tunnel_id " ID"
+set the tunnel id of the tunnel to be shown. If not specified,
+information about all tunnels is printed.
+.SS ip l2tp add session - add a new session to a tunnel
+.TP
+.BI name " NAME "
+sets the session network interface name. Default is l2tpethN.
+.TP
+.BI tunnel_id " ID"
+set the tunnel id, which is a 32-bit integer value. Uniquely
+identifies the tunnel into which the session will be created. The
+tunnel must already exist.
+.TP
+.BI session_id " ID"
+set the session id, which is a 32-bit integer value. Uniquely
+identifies the session being created. The value used must match the
+peer_session_id value being used at the peer.
+.TP
+.BI peer_session_id " ID"
+set the peer session id, which is a 32-bit integer value assigned to
+the session by the peer. The value used must match the session_id
+value being used at the peer.
+.TP
+.BI cookie " HEXSTR"
+sets an optional cookie value to be assigned to the session. This is a
+4 or 8 byte value, specified as 8 or 16 hex digits,
+e.g. 014d3636deadbeef. The value must match the peer_cookie value set
+at the peer. The cookie value is carried in L2TP data packets and is
+checked for expected value at the peer. Default is to use no cookie.
+.TP
+.BI peer_cookie " HEXSTR"
+sets an optional peer cookie value to be assigned to the session. This
+is a 4 or 8 byte value, specified as 8 or 16 hex digits,
+e.g. 014d3636deadbeef. The value must match the cookie value set at
+the peer. It tells the local system what cookie value to expect to
+find in received L2TP packets. Default is to use no cookie.
+.TP
+.BI offset " OFFSET"
+sets the byte offset from the L2TP header where user data starts in
+transmitted L2TP data packets. This is hardly ever used. If set, the
+value must match the peer_offset value used at the peer. Default is 0.
+.TP
+.BI peer_offset " OFFSET"
+sets the byte offset from the L2TP header where user data starts in
+received L2TP data packets. This is hardly ever used. If set, the
+value must match the offset value used at the peer. Default is 0.
+.SS ip l2tp del session - destroy a session
+.TP
+.BI tunnel_id " ID"
+set the tunnel id in which the session to be deleted is located.
+.TP
+.BI session_id " ID"
+set the session id of the session to be deleted.
+.SS ip l2tp show session - show information about sessions
+.TP
+.BI tunnel_id " ID"
+set the tunnel id of the session(s) to be shown. If not specified,
+information about sessions in all tunnels is printed.
+.TP
+.BI session_id " ID"
+set the session id of the session to be shown. If not specified,
+information about all sessions is printed.
+.SH EXAMPLES
+.PP
+.SS Setup L2TP tunnels and sessions
+.nf
+site-A:# ip l2tp add tunnel tunnel_id 3000 peer_tunnel_id 4000 \\
+           encap udp local 1.2.3.4 remote 5.6.7.8 \\
+           udp_sport 5000 udp_dport 6000
+site-A:# ip l2tp add session tunnel_id 3000 session_id 1000 \\
+           peer_session_id 2000
+
+site-B:# ip l2tp add tunnel tunnel_id 4000 peer_tunnel_id 3000 \\
+           encap udp local 5.6.7.8 remote 1.2.3.4 \\
+           udp_sport 6000 udp_dport 5000
+site-B:# ip l2tp add session tunnel_id 4000 session_id 2000 \\
+           peer_session_id 1000
+
+site-A:# ip link set l2tpeth0 up mtu 1488
+
+site-B:# ip link set l2tpeth0 up mtu 1488
+.fi
+.PP
+Notice that the IP addresses, UDP ports and tunnel / session ids are
+matched and reversed at each site.
+.SS Configure as IP interfaces
+The two interfaces can be configured with IP addresses if only IP data
+is to be carried. This is perhaps the simplest configuration.
+.PP
+.nf
+site-A:# ip addr add 10.42.1.1 peer 10.42.1.2 dev l2tpeth0
+
+site-B:# ip addr add 10.42.1.2 peer 10.42.1.1 dev l2tpeth0
+
+site-A:# ping 10.42.1.2
+.fi
+.PP
+Now the link should be usable. Add static routes as needed to have
+data sent over the new link.
+.PP
+.SS Configure as bridged interfaces
+To carry non-IP data, the L2TP network interface is added to a bridge
+instead of being assigned its own IP address, using standard Linux
+utilities.
+.PP
+.nf
+site-A:# brctl addbr br0
+site-A:# brctl addif br0 l2tpeth0
+site-A:# brctl addif br0 eth0
+site-A:# ip link set br0 up promisc on
+.fi
+.PP
+If you are using VLANs, setup a bridge per VLAN and bridge each VLAN
+over a separate L2TP session. For example, to bridge VLAN ID 5 on eth1
+over an L2TP pseudowire:
+.PP
+.nf
+site-A:# brctl addbr brvlan5
+site-A:# brctl addif brvlan5 l2tpeth0.5
+site-A:# brctl addif brvlan5 eth1.5
+site-A:# ip link set brvlan5 up promisc on
+.fi
+.PP
+Adding the L2TP interface to a bridge causes the bridge to forward
+traffic over the L2TP pseudowire just like it forwards over any other
+interface. The bridge learns MAC addresses of hosts attached to each
+interface and intelligently forwards frames from one bridge port to
+another. IP addresses are not assigned to the l2tpethN interfaces. If
+the bridge is correctly configured at both sides of the L2TP
+pseudowire, it should be possible to reach hosts in the peer's bridged
+network.
+.SH INTEROPABILITY
+.PP
+Unmanaged (static) L2TPv3 tunnels are supported by some network
+equipment equipment vendors such as Cisco.
+.PP
+In Linux, L2TP Hello messages are not supported in unmanaged
+tunnels. Hello messages are used by L2TP clients and servers to detect
+link failures in order to automate tearing down and reestablishing
+dynamic tunnels. If a non-Linux peer supports Hello messages in
+unmanaged tunnels, it must be turned off to interoperate with Linux.
+.SH SEE ALSO
+.br
+.BR ip (8)
+.SH AUTHOR
+James Chapman <jchapman@katalix.com>
diff --git a/man/man8/ip.8 b/man/man8/ip.8
index 0f9f454..ede3d12 100644
--- a/man/man8/ip.8
+++ b/man/man8/ip.8
@@ -113,7 +113,7 @@ host addresses.
 
 .TP
 .B l2tp
-- tunnel PPP over IP (L2TP).
+- tunnel ethernet over IP (L2TPv3).
 
 .TP
 .B link
@@ -205,6 +205,7 @@ was written by Alexey N. Kuznetsov and added in Linux 2.2.
 .SH SEE ALSO
 .BR ip-address (8),
 .BR ip-addrlabel (8),
+.BR ip-l2tp (8),
 .BR ip-link (8),
 .BR ip-maddress (8),
 .BR ip-monitor (8),
-- 
1.7.0.4

^ permalink raw reply related

* [PATCH 0/2] iproute2: add support for L2TP over IPv6, add manpage
From: James Chapman @ 2012-04-20 11:29 UTC (permalink / raw)
  To: netdev

The "ip l2tp" commands already support L2TP over IPv4. These patches
add support for L2TP over IPv6 and add a man page covering the command
set.

The patches depend on the L2TP IPv6 patch series already submitted for
review.

Chris Elston (1):
  iproute2: allow IPv6 addresses for l2tp local and remote parameters

James Chapman (1):
  iproute2: add ip-l2tp man page

 ip/ipl2tp.c        |   59 +++++++--
 man/man8/ip-l2tp.8 |  350 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 man/man8/ip.8      |    3 +-
 3 files changed, 397 insertions(+), 15 deletions(-)
 create mode 100644 man/man8/ip-l2tp.8

^ permalink raw reply

* [PATCH 1/2] iproute2: allow IPv6 addresses for l2tp local and remote parameters
From: James Chapman @ 2012-04-20 11:29 UTC (permalink / raw)
  To: netdev; +Cc: Chris Elston
In-Reply-To: <1334921383-32109-1-git-send-email-jchapman@katalix.com>

From: Chris Elston <celston@katalix.com>

Adds support for parsing IPv6 addresses to the parameters local and
remote in the l2tp commands. Requires netlink attributes L2TP_ATTR_IP6_SADDR
and L2TP_ATTR_IP6_DADDR, added in a required kernel patch already submitted
to netdev.

Also enables printing of IPv6 addresses returned by the L2TP_CMD_TUNNEL_GET
request.

Signed-off-by: Chris Elston <celston@katalix.com>
Signed-off-by: James Chapman <jchapman@katalix.com>
---
 ip/ipl2tp.c |   59 +++++++++++++++++++++++++++++++++++++++++++++--------------
 1 files changed, 45 insertions(+), 14 deletions(-)

diff --git a/ip/ipl2tp.c b/ip/ipl2tp.c
index c5683f5..a05e1a3 100644
--- a/ip/ipl2tp.c
+++ b/ip/ipl2tp.c
@@ -50,8 +50,8 @@ struct l2tp_parm {
 	uint8_t cookie[8];
 	int peer_cookie_len;
 	uint8_t peer_cookie[8];
-	struct in_addr local_ip;
-	struct in_addr peer_ip;
+	inet_prefix local_ip;
+	inet_prefix peer_ip;
 
 	uint16_t pw_type;
 	uint16_t mtu;
@@ -97,6 +97,8 @@ static int create_tunnel(struct l2tp_parm *p)
 		struct genlmsghdr	g;
 		char   			buf[1024];
 	} req;
+	uint32_t local_attr = L2TP_ATTR_IP_SADDR;
+	uint32_t peer_attr = L2TP_ATTR_IP_DADDR;
 
 	memset(&req, 0, sizeof(req));
 	req.n.nlmsg_type = genl_family;
@@ -110,8 +112,14 @@ static int create_tunnel(struct l2tp_parm *p)
 	addattr8(&req.n, 1024, L2TP_ATTR_PROTO_VERSION, 3);
 	addattr16(&req.n, 1024, L2TP_ATTR_ENCAP_TYPE, p->encap);
 
-	addattr32(&req.n, 1024, L2TP_ATTR_IP_SADDR, p->local_ip.s_addr);
-	addattr32(&req.n, 1024, L2TP_ATTR_IP_DADDR, p->peer_ip.s_addr);
+	if (p->local_ip.family == AF_INET6)
+		local_attr = L2TP_ATTR_IP6_SADDR;
+	addattr_l(&req.n, 1024, local_attr, &p->local_ip.data, p->local_ip.bytelen);
+
+	if (p->peer_ip.family == AF_INET6)
+		peer_attr = L2TP_ATTR_IP6_DADDR;
+	addattr_l(&req.n, 1024, peer_attr, &p->peer_ip.data, p->peer_ip.bytelen);
+
 	if (p->encap == L2TP_ENCAPTYPE_UDP) {
 		addattr16(&req.n, 1024, L2TP_ATTR_UDP_SPORT, p->local_udp_port);
 		addattr16(&req.n, 1024, L2TP_ATTR_UDP_DPORT, p->peer_udp_port);
@@ -225,13 +233,14 @@ static void print_cookie(char *name, const uint8_t *cookie, int len)
 static void print_tunnel(const struct l2tp_data *data)
 {
 	const struct l2tp_parm *p = &data->config;
+	char buf[INET6_ADDRSTRLEN];
 
 	printf("Tunnel %u, encap %s\n",
 	       p->tunnel_id,
 	       p->encap == L2TP_ENCAPTYPE_UDP ? "UDP" :
 	       p->encap == L2TP_ENCAPTYPE_IP ? "IP" : "??");
-	printf("  From %s ", inet_ntoa(p->local_ip));
-	printf("to %s\n", inet_ntoa(p->peer_ip));
+	printf("  From %s ", inet_ntop(p->local_ip.family, p->local_ip.data, buf, sizeof(buf)));
+	printf("to %s\n", inet_ntop(p->peer_ip.family, p->peer_ip.data, buf, sizeof(buf)));
 	printf("  Peer tunnel %u\n",
 	       p->peer_tunnel_id);
 
@@ -315,10 +324,30 @@ static int get_response(struct nlmsghdr *n, void *arg)
 
 	if (attrs[L2TP_ATTR_RECV_TIMEOUT])
 		p->reorder_timeout = rta_getattr_u64(attrs[L2TP_ATTR_RECV_TIMEOUT]);
-	if (attrs[L2TP_ATTR_IP_SADDR])
-		p->local_ip.s_addr = rta_getattr_u32(attrs[L2TP_ATTR_IP_SADDR]);
-	if (attrs[L2TP_ATTR_IP_DADDR])
-		p->peer_ip.s_addr = rta_getattr_u32(attrs[L2TP_ATTR_IP_DADDR]);
+	if (attrs[L2TP_ATTR_IP_SADDR]) {
+		p->local_ip.family = AF_INET;
+		p->local_ip.data[0] = rta_getattr_u32(attrs[L2TP_ATTR_IP_SADDR]);
+		p->local_ip.bytelen = 4;
+		p->local_ip.bitlen = -1;
+	}
+	if (attrs[L2TP_ATTR_IP_DADDR]) {
+		p->peer_ip.family = AF_INET;
+		p->peer_ip.data[0] = rta_getattr_u32(attrs[L2TP_ATTR_IP_DADDR]);
+		p->peer_ip.bytelen = 4;
+		p->peer_ip.bitlen = -1;
+	}
+	if (attrs[L2TP_ATTR_IP6_SADDR]) {
+		p->local_ip.family = AF_INET6;
+		memcpy(&p->local_ip.data, RTA_DATA(attrs[L2TP_ATTR_IP6_SADDR]),
+			p->local_ip.bytelen = 16);
+		p->local_ip.bitlen = -1;
+	}
+	if (attrs[L2TP_ATTR_IP6_DADDR]) {
+		p->peer_ip.family = AF_INET6;
+		memcpy(&p->peer_ip.data, RTA_DATA(attrs[L2TP_ATTR_IP6_DADDR]),
+			p->peer_ip.bytelen = 16);
+		p->peer_ip.bitlen = -1;
+	}
 	if (attrs[L2TP_ATTR_UDP_SPORT])
 		p->local_udp_port = rta_getattr_u16(attrs[L2TP_ATTR_UDP_SPORT]);
 	if (attrs[L2TP_ATTR_UDP_DPORT])
@@ -529,10 +558,12 @@ static int parse_args(int argc, char **argv, int cmd, struct l2tp_parm *p)
 			p->ifname = *argv;
 		} else if (strcmp(*argv, "remote") == 0) {
 			NEXT_ARG();
-			p->peer_ip.s_addr = get_addr32(*argv);
+			if (get_addr(&p->peer_ip, *argv, AF_UNSPEC))
+				invarg("invalid remote address\n", *argv);
 		} else if (strcmp(*argv, "local") == 0) {
 			NEXT_ARG();
-			p->local_ip.s_addr = get_addr32(*argv);
+			if (get_addr(&p->local_ip, *argv, AF_UNSPEC))
+				invarg("invalid local address\n", *argv);
 		} else if ((strcmp(*argv, "tunnel_id") == 0) ||
 			   (strcmp(*argv, "tid") == 0)) {
 			__u32 uval;
@@ -648,10 +679,10 @@ static int do_add(int argc, char **argv)
 		missarg("peer_tunnel_id");
 
 	if (p.tunnel) {
-		if (p.local_ip.s_addr == 0)
+		if (p.local_ip.family == AF_UNSPEC)
 			missarg("local");
 
-		if (p.peer_ip.s_addr == 0)
+		if (p.peer_ip.family == AF_UNSPEC)
 			missarg("remote");
 
 		if (p.encap == L2TP_ENCAPTYPE_UDP) {
-- 
1.7.0.4

^ permalink raw reply related

* RE: [PATCH 0/9] atl1c: update hardware settings - v3
From: Huang, Xiong @ 2012-04-20 10:34 UTC (permalink / raw)
  To: davem@davemloft.net, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
  Cc: qca-linux-team, nic-devel
In-Reply-To: <1334902584-15584-1-git-send-email-xiong@qca.qualcomm.com>

Hi David

    Please ignore this patch set, we found some NICs sometimes may fail to get IP via DHCP, this issue doesn't 100% happen.
    So, after fixing it, we will re-send a new patch set.
    Any suggestion would be great appreciated. Thanks a lot !

-Xiong  

> -----Original Message-----
> From: Huang, Xiong
> Sent: Friday, April 20, 2012 14:16
> To: davem@davemloft.net; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Cc: qca-linux-team; nic-devel; Huang, Xiong
> Subject: [PATCH 0/9] atl1c: update hardware settings - v3
> 
> This update contains a serial of patches, most of them are hardware settings
> related. register definitions are refined (or removed if
> meaningless) for each patch.
> 
> This is the third serial patches, after reviewed/applied the fourth serial will be
> out.
> 
> The patches have addressed all sparse and checkpatch warnings.
> 
> Following NICs are tested:
> AR8131/AR8132/AR8151A/AR8152A/AR8152B
> Test item includes:
> build/install/uninstall/dhcp/ping/iperf/wol/reboot/etc.
> 
> xiong (9):
>   atl1c: remove REG_PHY_STATUS
>   atl1c: refine phy-register read/write function
>   atl1c: remove PHY contrl in atl1c_reset_pcie
>   atl1c: refine SERDES-clock related code
>   atl1c: remove PHY polling from atl1c_open
>   atl1c: update PHY reset related routine
>   atl1c: remove PHY reset/init for link down event
>   atl1c: add function atl1c_power_saving
>   atl1c: refine start/enable code for MAC module
> 
>  drivers/net/ethernet/atheros/atl1c/atl1c_ethtool.c |    2 +-
>  drivers/net/ethernet/atheros/atl1c/atl1c_hw.c      |  422 ++++++++++++-----
>  drivers/net/ethernet/atheros/atl1c/atl1c_hw.h      |  499 +++++++++++++-------
>  drivers/net/ethernet/atheros/atl1c/atl1c_main.c    |  240 +++-------
>  4 files changed, 693 insertions(+), 470 deletions(-)
> 
> --
> 1.7.7

^ permalink raw reply

* Re: fake rtable dst patch applied but kernel keeps panicing
From: Massimo Cetra @ 2012-04-20 10:02 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Massimo Cetra, netdev, peter.huangpeng
In-Reply-To: <1334825108.2395.28.camel@edumazet-glaptop>

On 19/04/2012 10:45, Eric Dumazet wrote:
> Oh well, at first glance nf_bridge_unshare() is buggy, not sure if this
> can help your bug, but its another step.
>
>
> [PATCH] bridge: fix nf_bridge_unshare()
>
> If memory allocation failed, return an error.
>
> If not, skb->nf_bridge should be updated to point to the copy, not old
> info, or bad things can happen.
>
> Signed-off-by: Eric Dumazet<eric.dumazet@gmail.com>
> ---
>   net/bridge/br_netfilter.c |   24 ++++++++++++------------
>   1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/net/bridge/br_netfilter.c b/net/bridge/br_netfilter.c
> index dec4f38..b7c2cec 100644
> --- a/net/bridge/br_netfilter.c
> +++ b/net/bridge/br_netfilter.c
> @@ -185,21 +185,20 @@ static inline struct nf_bridge_info *nf_bridge_alloc(struct sk_buff *skb)
>   	return skb->nf_bridge;
>   }
>
> -static inline struct nf_bridge_info *nf_bridge_unshare(struct sk_buff *skb)
> +
> +static inline int nf_bridge_unshare(struct sk_buff *skb)
>   {
> -	struct nf_bridge_info *nf_bridge = skb->nf_bridge;
> +	struct nf_bridge_info *copy, *nf_bridge = skb->nf_bridge;
>
>   	if (atomic_read(&nf_bridge->use)>  1) {
> -		struct nf_bridge_info *tmp = nf_bridge_alloc(skb);
> -
> -		if (tmp) {
> -			memcpy(tmp, nf_bridge, sizeof(struct nf_bridge_info));
> -			atomic_set(&tmp->use, 1);
> -		}
> +		copy = kmemdup(nf_bridge, sizeof(*nf_bridge), GFP_ATOMIC);
> +		if (!copy)
> +			return -ENOMEM;
> +		atomic_set(&copy->use, 1);
>   		nf_bridge_put(nf_bridge);
> -		nf_bridge = tmp;
> +		skb->nf_bridge = copy;
>   	}
> -	return nf_bridge;
> +	return 0;
>   }
>
>   static inline void nf_bridge_push_encap_header(struct sk_buff *skb)
> @@ -744,8 +743,9 @@ static unsigned int br_nf_forward_ip(unsigned int hook, struct sk_buff *skb,
>   		return NF_ACCEPT;
>
>   	/* Need exclusive nf_bridge_info since we might have multiple
> -	 * different physoutdevs. */
> -	if (!nf_bridge_unshare(skb))
> +	 * different physoutdevs.
> +	 */
> +	if (nf_bridge_unshare(skb))
>   		return NF_DROP;
>
>   	parent = bridge_parent(out);
>
>

Hello, Eric,

i applied this patch and Peters last patch to a 3.3.2 kernel.
The result was a bit disappointing because the step was backwards.

Locally, from the same machine, i could ping each IP of each tun 
interface used by any virtual server.

 From the LAN such addresses were not pingable while the ip address of 
the bridge was reachable.

Max

^ permalink raw reply

* [PATCH] Fix tx transfer padding for full-speed USB
From: Ingo van Lil @ 2012-04-20 10:00 UTC (permalink / raw)
  To: netdev; +Cc: Ingo van Lil

The asix.c USB Ethernet driver avoids ending a tx transfer with a zero-
length packet by appending a four-byte padding to transfers whose length
is a multiple of maxpacket. However, the hard-coded 512 byte maxpacket
length is valid for high-speed USB only; full-speed USB uses 64 byte
packets.

Signed-off-by: Ingo van Lil <inguin@gmx.de>
---
 drivers/net/usb/asix.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/usb/asix.c b/drivers/net/usb/asix.c
index 5ee032c..aeb2702 100644
--- a/drivers/net/usb/asix.c
+++ b/drivers/net/usb/asix.c
@@ -355,7 +355,7 @@ static struct sk_buff *asix_tx_fixup(struct usbnet *dev, struct sk_buff *skb,
 	u32 packet_len;
 	u32 padbytes = 0xffff0000;
 
-	padlen = ((skb->len + 4) % 512) ? 0 : 4;
+	padlen = ((skb->len + 4) % dev->maxpacket) ? 0 : 4;
 
 	if ((!skb_cloned(skb)) &&
 	    ((headroom + tailroom) >= (4 + padlen))) {
@@ -377,7 +377,7 @@ static struct sk_buff *asix_tx_fixup(struct usbnet *dev, struct sk_buff *skb,
 	cpu_to_le32s(&packet_len);
 	skb_copy_to_linear_data(skb, &packet_len, sizeof(packet_len));
 
-	if ((skb->len % 512) == 0) {
+	if (padlen) {
 		cpu_to_le32s(&padbytes);
 		memcpy(skb_tail_pointer(skb), &padbytes, sizeof(padbytes));
 		skb_put(skb, sizeof(padbytes));
-- 
1.7.7.6

^ permalink raw reply related

* [PATCH] smsc95xx: mark link down on startup and let PHY interrupt deal with carrier changes
From: Paolo Pisati @ 2012-04-20  9:35 UTC (permalink / raw)
  To: netdev

Without this patch sysfs reports the cable as present

flag@flag-desktop:~$ cat /sys/class/net/eth0/carrier
1

while it's not:

flag@flag-desktop:~$ sudo mii-tool eth0
eth0: no link

Signed-off-by: Paolo Pisati <paolo.pisati@canonical.com>
---
 drivers/net/usb/smsc95xx.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 3444514..93d3252 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -1192,7 +1192,7 @@ static const struct driver_info smsc95xx_info = {
 	.rx_fixup	= smsc95xx_rx_fixup,
 	.tx_fixup	= smsc95xx_tx_fixup,
 	.status		= smsc95xx_status,
-	.flags		= FLAG_ETHER | FLAG_SEND_ZLP,
+	.flags		= FLAG_ETHER | FLAG_SEND_ZLP | FLAG_LINK_INTR,
 };
 
 static const struct usb_device_id products[] = {
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH] gianfar: add GRO support
From: Jiajun Wu @ 2012-04-20  8:54 UTC (permalink / raw)
  To: netdev, davem; +Cc: linuxppc-dev, Jiajun Wu

Replace netif_receive_skb with napi_gro_receive.

Signed-off-by: Jiajun Wu <b06378@freescale.com>
---
 drivers/net/ethernet/freescale/gianfar.c |   13 +++++++------
 1 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index e7bed53..1adb024 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -136,7 +136,7 @@ static void gfar_netpoll(struct net_device *dev);
 int gfar_clean_rx_ring(struct gfar_priv_rx_q *rx_queue, int rx_work_limit);
 static int gfar_clean_tx_ring(struct gfar_priv_tx_q *tx_queue);
 static int gfar_process_frame(struct net_device *dev, struct sk_buff *skb,
-			      int amount_pull);
+			      int amount_pull, struct napi_struct *napi);
 void gfar_halt(struct net_device *dev);
 static void gfar_halt_nodisable(struct net_device *dev);
 void gfar_start(struct net_device *dev);
@@ -2675,12 +2675,12 @@ static inline void gfar_rx_checksum(struct sk_buff *skb, struct rxfcb *fcb)
 /* gfar_process_frame() -- handle one incoming packet if skb
  * isn't NULL.  */
 static int gfar_process_frame(struct net_device *dev, struct sk_buff *skb,
-			      int amount_pull)
+			      int amount_pull, struct napi_struct *napi)
 {
 	struct gfar_private *priv = netdev_priv(dev);
 	struct rxfcb *fcb = NULL;
 
-	int ret;
+	gro_result_t ret;
 
 	/* fcb is at the beginning if exists */
 	fcb = (struct rxfcb *)skb->data;
@@ -2719,9 +2719,9 @@ static int gfar_process_frame(struct net_device *dev, struct sk_buff *skb,
 		__vlan_hwaccel_put_tag(skb, fcb->vlctl);
 
 	/* Send the packet up the stack */
-	ret = netif_receive_skb(skb);
+	ret = napi_gro_receive(napi, skb);
 
-	if (NET_RX_DROP == ret)
+	if (GRO_DROP == ret)
 		priv->extra_stats.kernel_dropped++;
 
 	return 0;
@@ -2783,7 +2783,8 @@ int gfar_clean_rx_ring(struct gfar_priv_rx_q *rx_queue, int rx_work_limit)
 				skb_put(skb, pkt_len);
 				rx_queue->stats.rx_bytes += pkt_len;
 				skb_record_rx_queue(skb, rx_queue->qindex);
-				gfar_process_frame(dev, skb, amount_pull);
+				gfar_process_frame(dev, skb, amount_pull,
+						&rx_queue->grp->napi);
 
 			} else {
 				netif_warn(priv, rx_err, dev, "Missing skb!\n");
-- 
1.5.6.5

^ permalink raw reply related

* [PATCH 05/10] l2tp: pppol2tp_connect() handles ipv6 sockaddr variants
From: James Chapman @ 2012-04-20  9:02 UTC (permalink / raw)
  To: netdev; +Cc: bcrl
In-Reply-To: <1334912573-28804-1-git-send-email-jchapman@katalix.com>

Userspace uses connect() to associate a pppol2tp socket with a tunnel
socket. This needs to allow the caller to supply the new IPv6
sockaddr_pppol2tp structures if IPv6 is used.

Signed-off-by: James Chapman <jchapman@katalix.com>
---
 net/l2tp/l2tp_ppp.c |   36 ++++++++++++++++++++++++++++--------
 1 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c
index 27b9dec..9f2c421 100644
--- a/net/l2tp/l2tp_ppp.c
+++ b/net/l2tp/l2tp_ppp.c
@@ -628,7 +628,6 @@ static int pppol2tp_connect(struct socket *sock, struct sockaddr *uservaddr,
 {
 	struct sock *sk = sock->sk;
 	struct sockaddr_pppol2tp *sp = (struct sockaddr_pppol2tp *) uservaddr;
-	struct sockaddr_pppol2tpv3 *sp3 = (struct sockaddr_pppol2tpv3 *) uservaddr;
 	struct pppox_sock *po = pppox_sk(sk);
 	struct l2tp_session *session = NULL;
 	struct l2tp_tunnel *tunnel;
@@ -657,7 +656,13 @@ static int pppol2tp_connect(struct socket *sock, struct sockaddr *uservaddr,
 	if (sk->sk_user_data)
 		goto end; /* socket is already attached */
 
-	/* Get params from socket address. Handle L2TPv2 and L2TPv3 */
+	/* Get params from socket address. Handle L2TPv2 and L2TPv3.
+	 * This is nasty because there are different sockaddr_pppol2tp
+	 * structs for L2TPv2, L2TPv3, over IPv4 and IPv6. We use
+	 * the sockaddr size to determine which structure the caller
+	 * is using.
+	 */
+	peer_tunnel_id = 0;
 	if (sockaddr_len == sizeof(struct sockaddr_pppol2tp)) {
 		fd = sp->pppol2tp.fd;
 		tunnel_id = sp->pppol2tp.s_tunnel;
@@ -665,12 +670,31 @@ static int pppol2tp_connect(struct socket *sock, struct sockaddr *uservaddr,
 		session_id = sp->pppol2tp.s_session;
 		peer_session_id = sp->pppol2tp.d_session;
 	} else if (sockaddr_len == sizeof(struct sockaddr_pppol2tpv3)) {
+		struct sockaddr_pppol2tpv3 *sp3 =
+			(struct sockaddr_pppol2tpv3 *) sp;
 		ver = 3;
 		fd = sp3->pppol2tp.fd;
 		tunnel_id = sp3->pppol2tp.s_tunnel;
 		peer_tunnel_id = sp3->pppol2tp.d_tunnel;
 		session_id = sp3->pppol2tp.s_session;
 		peer_session_id = sp3->pppol2tp.d_session;
+	} else if (sockaddr_len == sizeof(struct sockaddr_pppol2tpin6)) {
+		struct sockaddr_pppol2tpin6 *sp6 =
+			(struct sockaddr_pppol2tpin6 *) sp;
+		fd = sp6->pppol2tp.fd;
+		tunnel_id = sp6->pppol2tp.s_tunnel;
+		peer_tunnel_id = sp6->pppol2tp.d_tunnel;
+		session_id = sp6->pppol2tp.s_session;
+		peer_session_id = sp6->pppol2tp.d_session;
+	} else if (sockaddr_len == sizeof(struct sockaddr_pppol2tpv3in6)) {
+		struct sockaddr_pppol2tpv3in6 *sp6 =
+			(struct sockaddr_pppol2tpv3in6 *) sp;
+		ver = 3;
+		fd = sp6->pppol2tp.fd;
+		tunnel_id = sp6->pppol2tp.s_tunnel;
+		peer_tunnel_id = sp6->pppol2tp.d_tunnel;
+		session_id = sp6->pppol2tp.s_session;
+		peer_session_id = sp6->pppol2tp.d_session;
 	} else {
 		error = -EINVAL;
 		goto end; /* bad socket address */
@@ -711,12 +735,8 @@ static int pppol2tp_connect(struct socket *sock, struct sockaddr *uservaddr,
 	if (tunnel->recv_payload_hook == NULL)
 		tunnel->recv_payload_hook = pppol2tp_recv_payload_hook;
 
-	if (tunnel->peer_tunnel_id == 0) {
-		if (ver == 2)
-			tunnel->peer_tunnel_id = sp->pppol2tp.d_tunnel;
-		else
-			tunnel->peer_tunnel_id = sp3->pppol2tp.d_tunnel;
-	}
+	if (tunnel->peer_tunnel_id == 0)
+		tunnel->peer_tunnel_id = peer_tunnel_id;
 
 	/* Create session if it doesn't already exist. We handle the
 	 * case where a session was previously created by the netlink
-- 
1.7.0.4

^ 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