Netdev List
 help / color / mirror / Atom feed
* [PATCH v2 net-next 2/2] tipc: add name distributor resiliency queue
From: erik.hugne @ 2014-08-26  8:57 UTC (permalink / raw)
  To: jon.maloy, ying.xue, richard.alpe, netdev; +Cc: tipc-discussion, Erik Hugne
In-Reply-To: <1409043477-22761-1-git-send-email-erik.hugne@ericsson.com>

From: Erik Hugne <erik.hugne@ericsson.com>

TIPC name table updates are distributed asynchronously in a cluster,
entailing a risk of certain race conditions. E.g., if two nodes
simultaneously issue conflicting (overlapping) publications, this may
not be detected until both publications have reached a third node, in
which case one of the publications will be silently dropped on that
node. Hence, we end up with an inconsistent name table.

In most cases this conflict is just a temporary race, e.g., one
node is issuing a publication under the assumption that a previous,
conflicting, publication has already been withdrawn by the other node.
However, because of the (rtt related) distributed update delay, this
may not yet hold true on all nodes. The symptom of this failure is a
syslog message: "tipc: Cannot publish {%u,%u,%u}, overlap error".

In this commit we add a resiliency queue at the receiving end of
the name table distributor. When insertion of an arriving publication
fails, we retain it in this queue for a short amount of time, assuming
that another update will arrive very soon and clear the conflict. If so
happens, we insert the publication, otherwise we drop it.

The (configurable) retention value defaults to 2000 ms. Knowing from
experience that the situation described above is extremely rare, there
is no risk that the queue will accumulate any large number of items.

Signed-off-by: Erik Hugne <erik.hugne@ericsson.com>
Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---

v2: Fixed phrasing and spelling in the sysctl documentation

 Documentation/sysctl/net.txt | 16 ++++++++++
 net/tipc/core.h              |  1 +
 net/tipc/name_distr.c        | 69 ++++++++++++++++++++++++++++++++++++++++++--
 net/tipc/name_distr.h        |  1 +
 net/tipc/name_table.c        |  8 ++---
 net/tipc/sysctl.c            |  7 +++++
 6 files changed, 95 insertions(+), 7 deletions(-)

diff --git a/Documentation/sysctl/net.txt b/Documentation/sysctl/net.txt
index 9a0319a..04892b8 100644
--- a/Documentation/sysctl/net.txt
+++ b/Documentation/sysctl/net.txt
@@ -241,6 +241,9 @@ address of the router (or Connected) for internal networks.
 6. TIPC
 -------------------------------------------------------
 
+tipc_rmem
+----------
+
 The TIPC protocol now has a tunable for the receive memory, similar to the
 tcp_rmem - i.e. a vector of 3 INTEGERs: (min, default, max)
 
@@ -252,3 +255,16 @@ The max value is set to CONN_OVERLOAD_LIMIT, and the default and min values
 are scaled (shifted) versions of that same value.  Note that the min value
 is not at this point in time used in any meaningful way, but the triplet is
 preserved in order to be consistent with things like tcp_rmem.
+
+named_timeout
+--------------
+
+TIPC name table updates are distributed asynchronously in a cluster, without
+any form of transaction handling. This means that different race scenarios are
+possible. One such is that a name withdrawal sent out by one node and received
+by another node may arrive after a second, overlapping name publication already
+has been accepted from a third node, although the conflicting updates
+originally may have been issued in the correct sequential order.
+If named_timeout is nonzero, failed topology updates will be placed on a defer
+queue until another event arrives that clears the error, or until the timeout
+expires. Value is in milliseconds.
diff --git a/net/tipc/core.h b/net/tipc/core.h
index d2607a8..f773b14 100644
--- a/net/tipc/core.h
+++ b/net/tipc/core.h
@@ -81,6 +81,7 @@ extern u32 tipc_own_addr __read_mostly;
 extern int tipc_max_ports __read_mostly;
 extern int tipc_net_id __read_mostly;
 extern int sysctl_tipc_rmem[3] __read_mostly;
+extern int sysctl_tipc_named_timeout __read_mostly;
 
 /*
  * Other global variables
diff --git a/net/tipc/name_distr.c b/net/tipc/name_distr.c
index 0591f33..0cbe5e1 100644
--- a/net/tipc/name_distr.c
+++ b/net/tipc/name_distr.c
@@ -1,7 +1,7 @@
 /*
  * net/tipc/name_distr.c: TIPC name distribution code
  *
- * Copyright (c) 2000-2006, Ericsson AB
+ * Copyright (c) 2000-2006, 2014, Ericsson AB
  * Copyright (c) 2005, 2010-2011, Wind River Systems
  * All rights reserved.
  *
@@ -71,6 +71,21 @@ static struct publ_list *publ_lists[] = {
 };
 
 
+int sysctl_tipc_named_timeout __read_mostly = 2000;
+
+/**
+ * struct tipc_dist_queue - queue holding deferred name table updates
+ */
+static struct list_head tipc_dist_queue = LIST_HEAD_INIT(tipc_dist_queue);
+
+struct distr_queue_item {
+	struct distr_item i;
+	u32 dtype;
+	u32 node;
+	u64 expiry;
+	struct list_head next;
+};
+
 /**
  * publ_to_item - add publication info to a publication message
  */
@@ -299,6 +314,52 @@ struct publication *tipc_update_nametbl(struct distr_item *i, u32 node,
 }
 
 /**
+ * tipc_named_add_backlog - add a failed name table update to the backlog
+ *
+ */
+static void tipc_named_add_backlog(struct distr_item *i, u32 type, u32 node)
+{
+	struct distr_queue_item *e;
+	u64 now = get_jiffies_64();
+
+	e = kzalloc(sizeof(*e), GFP_ATOMIC);
+	if (!e)
+		return;
+	e->dtype = type;
+	e->node = node;
+	e->expiry = now + msecs_to_jiffies(sysctl_tipc_named_timeout);
+	memcpy(e, i, sizeof(*i));
+	list_add_tail(&e->next, &tipc_dist_queue);
+}
+
+/**
+ * tipc_named_process_backlog - try to process any pending name table updates
+ * from the network.
+ */
+void tipc_named_process_backlog(void)
+{
+	struct distr_queue_item *e, *tmp;
+	char addr[16];
+	u64 now = get_jiffies_64();
+
+	list_for_each_entry_safe(e, tmp, &tipc_dist_queue, next) {
+		if (e->expiry > now) {
+			if (!tipc_update_nametbl(&e->i, e->node, e->dtype))
+				continue;
+		} else {
+			tipc_addr_string_fill(addr, e->node);
+			pr_warn_ratelimited("Dropping name table update (%d) of {%u, %u, %u} from %s key=%u\n",
+					    e->dtype, ntohl(e->i.type),
+					    ntohl(e->i.lower),
+					    ntohl(e->i.upper),
+					    addr, ntohl(e->i.key));
+		}
+		list_del(&e->next);
+		kfree(e);
+	}
+}
+
+/**
  * tipc_named_rcv - process name table update message sent by another node
  */
 void tipc_named_rcv(struct sk_buff *buf)
@@ -306,13 +367,15 @@ void tipc_named_rcv(struct sk_buff *buf)
 	struct tipc_msg *msg = buf_msg(buf);
 	struct distr_item *item = (struct distr_item *)msg_data(msg);
 	u32 count = msg_data_sz(msg) / ITEM_SIZE;
+	u32 node = msg_orignode(msg);
 
 	write_lock_bh(&tipc_nametbl_lock);
 	while (count--) {
-		tipc_update_nametbl(item, msg_orignode(msg),
-				    msg_type(msg));
+		if (!tipc_update_nametbl(item, node, msg_type(msg)))
+			tipc_named_add_backlog(item, msg_type(msg), node);
 		item++;
 	}
+	tipc_named_process_backlog();
 	write_unlock_bh(&tipc_nametbl_lock);
 	kfree_skb(buf);
 }
diff --git a/net/tipc/name_distr.h b/net/tipc/name_distr.h
index 8afe32b..b9e75fe 100644
--- a/net/tipc/name_distr.h
+++ b/net/tipc/name_distr.h
@@ -73,5 +73,6 @@ void named_cluster_distribute(struct sk_buff *buf);
 void tipc_named_node_up(u32 dnode);
 void tipc_named_rcv(struct sk_buff *buf);
 void tipc_named_reinit(void);
+void tipc_named_process_backlog(void);
 
 #endif
diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c
index c058e30..3a6a0a7 100644
--- a/net/tipc/name_table.c
+++ b/net/tipc/name_table.c
@@ -261,8 +261,6 @@ static struct publication *tipc_nameseq_insert_publ(struct name_seq *nseq,
 
 		/* Lower end overlaps existing entry => need an exact match */
 		if ((sseq->lower != lower) || (sseq->upper != upper)) {
-			pr_warn("Cannot publish {%u,%u,%u}, overlap error\n",
-				type, lower, upper);
 			return NULL;
 		}
 
@@ -284,8 +282,6 @@ static struct publication *tipc_nameseq_insert_publ(struct name_seq *nseq,
 		/* Fail if upper end overlaps into an existing entry */
 		if ((inspos < nseq->first_free) &&
 		    (upper >= nseq->sseqs[inspos].lower)) {
-			pr_warn("Cannot publish {%u,%u,%u}, overlap error\n",
-				type, lower, upper);
 			return NULL;
 		}
 
@@ -677,6 +673,8 @@ struct publication *tipc_nametbl_publish(u32 type, u32 lower, u32 upper,
 	if (likely(publ)) {
 		table.local_publ_count++;
 		buf = tipc_named_publish(publ);
+		/* Any pending external events? */
+		tipc_named_process_backlog();
 	}
 	write_unlock_bh(&tipc_nametbl_lock);
 
@@ -698,6 +696,8 @@ int tipc_nametbl_withdraw(u32 type, u32 lower, u32 ref, u32 key)
 	if (likely(publ)) {
 		table.local_publ_count--;
 		buf = tipc_named_withdraw(publ);
+		/* Any pending external events? */
+		tipc_named_process_backlog();
 		write_unlock_bh(&tipc_nametbl_lock);
 		list_del_init(&publ->pport_list);
 		kfree(publ);
diff --git a/net/tipc/sysctl.c b/net/tipc/sysctl.c
index f3fef93..1a779b1 100644
--- a/net/tipc/sysctl.c
+++ b/net/tipc/sysctl.c
@@ -47,6 +47,13 @@ static struct ctl_table tipc_table[] = {
 		.mode		= 0644,
 		.proc_handler	= proc_dointvec,
 	},
+	{
+		.procname	= "named_timeout",
+		.data		= &sysctl_tipc_named_timeout,
+		.maxlen		= sizeof(sysctl_tipc_named_timeout),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec,
+	},
 	{}
 };
 
-- 
1.8.3.2

^ permalink raw reply related

* [PATCH v2 net-next 1/2] tipc: refactor name table updates out of named packet receive routine
From: erik.hugne @ 2014-08-26  8:57 UTC (permalink / raw)
  To: jon.maloy, ying.xue, richard.alpe, netdev; +Cc: tipc-discussion, Erik Hugne

From: Erik Hugne <erik.hugne@ericsson.com>

We need to perform the same actions when processing deferred name
table updates, so this functionality is moved to a separate
function.

Signed-off-by: Erik Hugne <erik.hugne@ericsson.com>
Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---
 net/tipc/name_distr.c | 74 ++++++++++++++++++++++++++-------------------------
 1 file changed, 38 insertions(+), 36 deletions(-)

diff --git a/net/tipc/name_distr.c b/net/tipc/name_distr.c
index dcc15bc..0591f33 100644
--- a/net/tipc/name_distr.c
+++ b/net/tipc/name_distr.c
@@ -263,52 +263,54 @@ static void named_purge_publ(struct publication *publ)
 }
 
 /**
+ * tipc_update_nametbl - try to process a nametable update and notify
+ *			 subscribers
+ *
+ * tipc_nametbl_lock must be held.
+ * Returns the publication item if successful, otherwise NULL.
+ */
+struct publication *tipc_update_nametbl(struct distr_item *i, u32 node,
+					u32 dtype)
+{
+	struct publication *publ = NULL;
+
+	if (dtype == PUBLICATION) {
+		publ = tipc_nametbl_insert_publ(ntohl(i->type), ntohl(i->lower),
+						ntohl(i->upper),
+						TIPC_CLUSTER_SCOPE, node,
+						ntohl(i->ref), ntohl(i->key));
+		if (publ) {
+			tipc_nodesub_subscribe(&publ->subscr, node, publ,
+					       (net_ev_handler)
+					       named_purge_publ);
+		}
+	} else if (dtype == WITHDRAWAL) {
+		publ = tipc_nametbl_remove_publ(ntohl(i->type), ntohl(i->lower),
+						node, ntohl(i->ref),
+						ntohl(i->key));
+		if (publ) {
+			tipc_nodesub_unsubscribe(&publ->subscr);
+			kfree(publ);
+		}
+	} else {
+		pr_warn("Unrecognized name table message received\n");
+	}
+	return publ;
+}
+
+/**
  * tipc_named_rcv - process name table update message sent by another node
  */
 void tipc_named_rcv(struct sk_buff *buf)
 {
-	struct publication *publ;
 	struct tipc_msg *msg = buf_msg(buf);
 	struct distr_item *item = (struct distr_item *)msg_data(msg);
 	u32 count = msg_data_sz(msg) / ITEM_SIZE;
 
 	write_lock_bh(&tipc_nametbl_lock);
 	while (count--) {
-		if (msg_type(msg) == PUBLICATION) {
-			publ = tipc_nametbl_insert_publ(ntohl(item->type),
-							ntohl(item->lower),
-							ntohl(item->upper),
-							TIPC_CLUSTER_SCOPE,
-							msg_orignode(msg),
-							ntohl(item->ref),
-							ntohl(item->key));
-			if (publ) {
-				tipc_nodesub_subscribe(&publ->subscr,
-						       msg_orignode(msg),
-						       publ,
-						       (net_ev_handler)
-						       named_purge_publ);
-			}
-		} else if (msg_type(msg) == WITHDRAWAL) {
-			publ = tipc_nametbl_remove_publ(ntohl(item->type),
-							ntohl(item->lower),
-							msg_orignode(msg),
-							ntohl(item->ref),
-							ntohl(item->key));
-
-			if (publ) {
-				tipc_nodesub_unsubscribe(&publ->subscr);
-				kfree(publ);
-			} else {
-				pr_err("Unable to remove publication by node 0x%x\n"
-				       " (type=%u, lower=%u, ref=%u, key=%u)\n",
-				       msg_orignode(msg), ntohl(item->type),
-				       ntohl(item->lower), ntohl(item->ref),
-				       ntohl(item->key));
-			}
-		} else {
-			pr_warn("Unrecognized name table message received\n");
-		}
+		tipc_update_nametbl(item, msg_orignode(msg),
+				    msg_type(msg));
 		item++;
 	}
 	write_unlock_bh(&tipc_nametbl_lock);
-- 
1.8.3.2

^ permalink raw reply related

* Re: [patch net-next RFC 03/12] net: introduce generic switch devices support
From: Jiri Pirko @ 2014-08-26  8:34 UTC (permalink / raw)
  To: Thomas Graf
  Cc: ryazanov.s.a-Re5JQEeQqe8AvxtiuMwx3w,
	jasowang-H+wXaHxf7aLQT0dZR+AlfA,
	john.r.fastabend-ral2JQCrhuEAvxtiuMwx3w,
	Neil.Jerram-QnUH15yq9NYqDJ6do+/SaQ,
	edumazet-hpIqsD4AKlfQT0dZR+AlfA, andy-QlMahl40kYEqcZcGjlUOXw,
	dev-yBygre7rU0TnMu66kgdUjQ, nbd-p3rKhJxN3npAfugRpC6u6w,
	f.fainelli-Re5JQEeQqe8AvxtiuMwx3w, ronye-VPRAkNaXOzVWk0Htik3J/w,
	jeffrey.t.kirsher-ral2JQCrhuEAvxtiuMwx3w,
	ogerlitz-VPRAkNaXOzVWk0Htik3J/w, ben-/+tVBieCtBitmTQ+vhA3Yw,
	buytenh-OLH4Qvv75CYX/NnBR394Jw,
	roopa-qUQiAmfTcIp+XZJcv9eMoEEOCMrvLtNR,
	jhs-jkUAjuhPggJWk0Htik3J/w, aviadr-VPRAkNaXOzVWk0Htik3J/w,
	nicolas.dichtel-pdR9zngts4EAvxtiuMwx3w,
	vyasevic-H+wXaHxf7aLQT0dZR+AlfA, nhorman-2XuSBdqkA4R54TAoqtyWWQ,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	stephen-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ,
	dborkman-H+wXaHxf7aLQT0dZR+AlfA, ebiederm-aS9lmoZGLiVWk0Htik3J/w,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <20140824114605.GC32741-FZi0V3Vbi30CUdFEqe4BF2D2FQJk+8+b@public.gmane.org>

Sun, Aug 24, 2014 at 01:46:05PM CEST, tgraf-G/eBtMaohhA@public.gmane.org wrote:
>On 08/21/14 at 06:18pm, Jiri Pirko wrote:
>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>> index 39294b9..8b5d14c 100644
>> --- a/include/linux/netdevice.h
>> +++ b/include/linux/netdevice.h
>> @@ -49,6 +49,8 @@
>>  
>>  #include <linux/netdev_features.h>
>>  #include <linux/neighbour.h>
>> +#include <linux/sw_flow.h>
>> +
>>  #include <uapi/linux/netdevice.h>
>>  
>>  struct netpoll_info;
>> @@ -997,6 +999,24 @@ typedef u16 (*select_queue_fallback_t)(struct net_device *dev,
>> + * int (*ndo_swdev_flow_insert)(struct net_device *dev,
>> + *				const struct sw_flow *flow);
>> + *	Called to insert a flow into switch device. If driver does
>> + *	not implement this, it is assumed that the hw does not have
>> + *	a capability to work with flows.
>
>I asume you are planning to add an additional expandable struct
>paramter to handle insertion parameters when the first is introduced
>to avoid requiring to touch every driver every time.

Sure. That is the way to go.

>
>> +/**
>> + *	swdev_flow_insert - Insert a flow into switch
>> + *	@dev: port device
>> + *	@flow: flow descriptor
>> + *
>> + *	Insert a flow into switch this port is part of.
>> + */
>> +int swdev_flow_insert(struct net_device *dev, const struct sw_flow *flow)
>> +{
>> +	const struct net_device_ops *ops = dev->netdev_ops;
>> +
>> +	print_flow(flow, dev, "insert");
>> +	if (!ops->ndo_swdev_flow_insert)
>> +		return -EOPNOTSUPP;
>> +	WARN_ON(!ops->ndo_swdev_get_id);
>> +	BUG_ON(!flow->actions);
>> +	return ops->ndo_swdev_flow_insert(dev, flow);
>> +}
>> +EXPORT_SYMBOL(swdev_flow_insert);
>
>Splitting the flow specific API into a separate file (maybe
>swdev_flow.c?) might help resolve some of the concerns around the
>focus on flows. It would make it clear that it's one of multiple
>models to be supported.

I understand your point. But the file is tiny as it is. I would keep all
in one file for now.

^ permalink raw reply

* Re: [patch net-next RFC 04/12] rtnl: expose physical switch id for particular device
From: Jiri Pirko @ 2014-08-26  8:32 UTC (permalink / raw)
  To: John Fastabend
  Cc: ryazanov.s.a-Re5JQEeQqe8AvxtiuMwx3w,
	jasowang-H+wXaHxf7aLQT0dZR+AlfA,
	john.r.fastabend-ral2JQCrhuEAvxtiuMwx3w,
	Neil.Jerram-QnUH15yq9NYqDJ6do+/SaQ,
	edumazet-hpIqsD4AKlfQT0dZR+AlfA, andy-QlMahl40kYEqcZcGjlUOXw,
	dev-yBygre7rU0TnMu66kgdUjQ, nbd-p3rKhJxN3npAfugRpC6u6w,
	f.fainelli-Re5JQEeQqe8AvxtiuMwx3w, ronye-VPRAkNaXOzVWk0Htik3J/w,
	jeffrey.t.kirsher-ral2JQCrhuEAvxtiuMwx3w,
	ogerlitz-VPRAkNaXOzVWk0Htik3J/w, ben-/+tVBieCtBitmTQ+vhA3Yw,
	buytenh-OLH4Qvv75CYX/NnBR394Jw,
	roopa-qUQiAmfTcIp+XZJcv9eMoEEOCMrvLtNR,
	jhs-jkUAjuhPggJWk0Htik3J/w, aviadr-VPRAkNaXOzVWk0Htik3J/w,
	nicolas.dichtel-pdR9zngts4EAvxtiuMwx3w,
	vyasevic-H+wXaHxf7aLQT0dZR+AlfA, nhorman-2XuSBdqkA4R54TAoqtyWWQ,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	stephen-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ,
	dborkman-H+wXaHxf7aLQT0dZR+AlfA, ebiederm-aS9lmoZGLiVWk0Htik3J/w,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <53F79537.20207-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Fri, Aug 22, 2014 at 09:08:39PM CEST, john.fastabend-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:
>On 08/21/2014 09:18 AM, Jiri Pirko wrote:
>>The netdevice represents a port in a switch, it will expose
>>IFLA_PHYS_SWITCH_ID value via rtnl. Two netdevices with the same value
>>belong to one physical switch.
>>
>>Signed-off-by: Jiri Pirko <jiri-rHqAuBHg3fBzbRFIqnYvSA@public.gmane.org>
>
>What is the relation between phys_port_id and phys_switch_id?
>
>phys_port_id was intended to identify a set of ports that belong
>to a single uplink port,
>
>
>	eth0     eth1    eth2   eth3      (host facing)
>          |       |       |      |
>          |       |       |      |
>      +---+-------+-------+------+---+
>      |      embedded switch         |
>      +------------------------------+
>                     |
>                    MAC                   (network)
>
>In the NIC case there is a simply switch with a port to the
>network which we currently don't represent with a netdev. Any
>netdev where the phys_switch_id's are behind the same embedded
>switch.

I think that MAC in your picture should be represented as netdev (switch
port). Also, the other ports connected to eth0-3 should be represented
as netdevs. All of these + the MAC should have the same switch id.

>
>In the switch id case we are indicating the port is attached to
>the same embedded switch as well.
>
>         eth0 eth1 eth2 eth3
>          |    |    |    |
>     +----+----+----+----+----+
>     |         switch         |
>     +----+----+----+----+----+
>
>but they do not share an uplink port? So in this case each ethx
>has a unique phys_port_id but the same phys_switch_id?

Yes.

>
>In the first case both phys_port_id and phys_switch_id should
>be equal for all interfaces correct?

See above. In case of embedded switch on nic I believe that eth0-eth3
shoud have the same port_id and no switch_id as they are not ports of
switch (the counterparts in switch (marked as "+" on your picture are
the switch ports)

>
>Is that clear/useful at all? We need to document this somewhere
>if/when the patches are submitted otherwise I doubt we will get it
>consistently right across drivers. There could for example be
>somewhat strange devices with virtual functions hanging off of the
>switch.

I will extend the documentation to my "net: introduce generic switch
devices support" patch.

>
>Thanks,
>John
>
>-- 
>John Fastabend         Intel Corporation

^ permalink raw reply

* Re: [PATCH] net: stmmac: fix warning from Sparse for socfpga
From: Ley Foon Tan @ 2014-08-26  8:11 UTC (permalink / raw)
  To: Giuseppe CAVALLARO
  Cc: netdev, linux-kernel@vger.kernel.org, David S. Miller,
	Vince Bridgers
In-Reply-To: <53FC3F89.9030406@st.com>

On Sel, 2014-08-26 at 10:04 +0200, Giuseppe CAVALLARO wrote:

> 
> >
> >>
> >> patch should be for net-next
> > Do you mean the patch need based on net-next git?
> 
> yes I do.
> 
> > I'm using linux-next git now.
> 
> ok, can you signal it in the subject (e.g.  [PATCH (net-next)]
> This can help on reviewing and IIRC required by Maintainer too
Sure, will do it in next revision. 

Thanks.

Regards
Ley Foon

^ permalink raw reply

* Re: [RFC PATCH net-next] ipv6: stop sending PTB packets for MTU < 1280
From: Hagen Paul Pfeifer @ 2014-08-26  8:06 UTC (permalink / raw)
  To: Hannes Frederic Sowa; +Cc: netdev, Fernando Gont
In-Reply-To: <1409006842.6274.69.camel@localhost>

On 26 August 2014 00:47, Hannes Frederic Sowa
<hannes@stressinduktion.org> wrote:

Hey Hannes

> I wonder if we should wait until this gets RFC status?

Yes, we should wait until we get Fernando's go (based on v6ops
discussion & consensus). But the discussions seem to boils down to
this action (remove IPv6 PTB generation < 1280). If so we probably
don't want to wait two years with an open "protocol bug".

> I very much welcome this decision! I already raised this problem some
> time ago:
> http://lists.openwall.net/netdev/2013/12/31/17

Thank you for the pointer

> I wonder if we should add a mode alike ipv4 ip_no_pmtu_disc mode for
> ipv6:

Not sure, we should discuss this here (netdev). I tend to remove the
functionality completely. Any use cases where it makes sense to keep
this?

> This patch is a starter, yes. We can now get rid of the dst_allfrag
> function altogether.

Yes right, I am aware of this. Depending on the discussion we should
get rid of the allfrags code or not. Let's see. Thank you Hannes for
the comments.

Hagen

^ permalink raw reply

* Re: [PATCH] net: stmmac: fix warning from Sparse for socfpga
From: Giuseppe CAVALLARO @ 2014-08-26  8:04 UTC (permalink / raw)
  To: Ley Foon Tan
  Cc: netdev, linux-kernel@vger.kernel.org, David S. Miller,
	Vince Bridgers
In-Reply-To: <CAFiDJ5_LspyDPnXnimZ=TRiZD191XQRvwqgfZSOU-ziu_DUx=Q@mail.gmail.com>

On 8/26/2014 9:47 AM, Ley Foon Tan wrote:
> On Tue, Aug 26, 2014 at 3:24 PM, Giuseppe CAVALLARO
> <peppe.cavallaro@st.com> wrote:
>>> @@ -119,7 +119,8 @@ static int socfpga_dwmac_parse_data(struct
>>> socfpga_dwmac *dwmac, struct device *
>>>                          return -EINVAL;
>>>                  }
>>>
>>> -               dwmac->splitter_base = (void *)devm_ioremap_resource(dev,
>>> +               dwmac->splitter_base =
>>> +                       (void __iomem *)devm_ioremap_resource(dev,
>>
>>
>> I think, no casting should be done:
>>
>>     dwmac->splitter_base = devm_ioremap_resource(dev, ....
> Oh ya, since both are same type. Will send new patch.

thx a lot

>
>>
>> patch should be for net-next
> Do you mean the patch need based on net-next git?

yes I do.

> I'm using linux-next git now.

ok, can you signal it in the subject (e.g.  [PATCH (net-next)]
This can help on reviewing and IIRC required by Maintainer too

>
> Thanks.

welcome

BR
peppe

> Regards
> Ley Foon
>
>

^ permalink raw reply

* Re: [PATCH v6 net-next 4/6] bpf: enable bpf syscall on x64 and i386
From: Ingo Molnar @ 2014-08-26  8:02 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: Alexei Starovoitov, Stephen Hemminger, David S. Miller,
	Linus Torvalds, Andy Lutomirski, Steven Rostedt, Chema Gonzalez,
	Eric Dumazet, Peter Zijlstra, Brendan Gregg, Namhyung Kim,
	H. Peter Anvin, Andrew Morton, Kees Cook, Linux API,
	Network Development, LKML
In-Reply-To: <53FC3E9A.1020108-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>


* Daniel Borkmann <dborkman-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:

> On 08/26/2014 09:46 AM, Ingo Molnar wrote:
> >* Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org> wrote:
> >>On Mon, Aug 25, 2014 at 8:52 PM, Stephen Hemminger
> >><stephen-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ@public.gmane.org> wrote:
> >>>Per discussion at Kernel Summit. Every new syscall requires
> >>>a manual page and test programs. We have had too many new syscalls
> >>>that are DOA.
> >>
> >>There is verifier testsuite that is testing eBPF verifier from userspace
> >>via bpf syscall. Also there are multiple examples and libbpf.
> >>I think test coverage for bpf syscall is quite substantial already.
> >
> >This is in tools/bpf/, right?
> 
> No, it contains a BPF JIT disasm, bpf assembler and a debugger, 
> but the last two are for the 'classic' BPF interface only. 
> There's a test suite for BPF/eBPF in general under 
> lib/test_bpf.c, but so far it tests only the current code w/o 
> eBPF verifier.
> 
> That said, I think Alexei is referring to the examples et al 
> from the bigger previous proposed patch set.

I mean, if all the testing already exists, it should be part of 
an initial submission and such.

Thanks,

	Ingo

^ permalink raw reply

* Re: [PATCH v6 net-next 4/6] bpf: enable bpf syscall on x64 and i386
From: Daniel Borkmann @ 2014-08-26  8:00 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Alexei Starovoitov, Stephen Hemminger, David S. Miller,
	Linus Torvalds, Andy Lutomirski, Steven Rostedt, Chema Gonzalez,
	Eric Dumazet, Peter Zijlstra, Brendan Gregg, Namhyung Kim,
	H. Peter Anvin, Andrew Morton, Kees Cook, Linux API,
	Network Development, LKML
In-Reply-To: <20140826074655.GB19799-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

On 08/26/2014 09:46 AM, Ingo Molnar wrote:
> * Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org> wrote:
>> On Mon, Aug 25, 2014 at 8:52 PM, Stephen Hemminger
>> <stephen-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ@public.gmane.org> wrote:
>>> Per discussion at Kernel Summit. Every new syscall requires
>>> a manual page and test programs. We have had too many new syscalls
>>> that are DOA.
>>
>> There is verifier testsuite that is testing eBPF verifier from userspace
>> via bpf syscall. Also there are multiple examples and libbpf.
>> I think test coverage for bpf syscall is quite substantial already.
>
> This is in tools/bpf/, right?

No, it contains a BPF JIT disasm, bpf assembler and a debugger, but the
last two are for the 'classic' BPF interface only. There's a test suite
for BPF/eBPF in general under lib/test_bpf.c, but so far it tests only
the current code w/o eBPF verifier.

That said, I think Alexei is referring to the examples et al from the
bigger previous proposed patch set.

^ permalink raw reply

* Aw: Re: Routes with unreachable gateways are staying in the routing table and they are functional
From: Fedor Babkin @ 2014-08-26  7:53 UTC (permalink / raw)
  To: Julian Anastasov; +Cc: netdev
In-Reply-To: <alpine.LFD.2.11.1408221856210.7852@ja.home.ssi.bg>

Hello Julian,

Thanks for your feedback. For IPv4 the situation is clear, you have to configure more that one subnet to the interface in order to start experiencing this issue. However exactly the same issue exists in IPv6, where once you configure a single address, you have to count on effectively having 2 addresses on the interface, due to the presence of a link-local address fe80::xxxx. Moreover with IPv6 stateless address autoconfiguration (RFC 4862), there is more address configuration dynamics in IPv6 comparing to IPv4. I would say this issue has a higher visibility and side-effect potential. Is there anyone looking into this issue from IPv6 perspective?

Thanks,
Fedor


	Hello,

On Fri, 22 Aug 2014, Fedor Babkin wrote:

> Hello,
> 
> I noticed that in case a network interface has two addresses assigned (no matter if IPv4 or IPv6) from subnets A and B, the route to reach subnet C via the gateway in A is staying configured and functional when subnet A is removed from the interface. Let me illustrate the behavior with IPv4 example.
> 
> Starting point. eth0 has 2 subnets, the route to reach 3-rd subnet via gw in one of them is present:
> eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UP group default qlen 1000
>     link/ether 52:54:00:12:34:57 brd ff:ff:ff:ff:ff:ff
>     inet 10.20.10.3/24 brd 10.20.10.255 scope global eth0
>        valid_lft forever preferred_lft forever
>     inet 10.30.10.3/24 brd 10.30.10.255 scope global eth0
>        valid_lft forever preferred_lft forever
>     inet6 fe80::5054:ff:fe12:3457/64 scope link
>        valid_lft forever preferred_lft forever
> root@...ntu14-4-vm:~# ip ro ls
> 10.11.10.0/24 via 10.30.10.1 dev eth0
> 10.20.10.0/24 dev eth0  proto kernel  scope link  src 10.20.10.3
> 10.30.10.0/24 dev eth0  proto kernel  scope link  src 10.30.10.3
> 
> Delete 10.30.10.3/24, the indirect route persists:
> root@...ntu14-4-vm:~# ip ro ls
> 10.11.10.0/24 via 10.30.10.1 dev eth0
> 10.20.10.0/24 dev eth0  proto kernel  scope link  src 10.20.10.3
> 
> Ping to destinations in 10.11.10.0/24 works, unreachable gateway 10.30.10.1 is resolved, even recovers in case of neighbor flushes.
> root@...ntu14-4-vm:~# ping 10.11.10.1
> PING 10.11.10.1 (10.11.10.1) 56(84) bytes of data.
> 64 bytes from 10.11.10.1: icmp_seq=1 ttl=64 time=0.706 ms
> root@...ntu14-4-vm:~# ip n l
> 10.30.10.1 dev eth0 lladdr 52:54:00:12:34:56 REACHABLE
> 
> The above was observed with kernel version 2.6.32 and confirmed with 3.13.0 In case of IPv6 this behavior is more disturbing as there is always LLA assigned, so applications have to orchestrate the removal of unreachable routes. 
> If it's an intended implementation, would be helpful to know a reason behind it. Thanks for your support.

	It is expensive to implement solution for this
problem. If we try to be very strict by handling it
at FIB level (catch route removal) simple operations
like secondary address promotion or route replacement
can lead to cascade of route removals. At the end,
it is again the user who have to take care to add
all lost routes back. So, it is a complex task to
solve.

	Here is recent discussion on such topic:

http://marc.info/?t=139030500700005&r=1&w=2
http://marc.info/?t=139055890700002&r=1&w=2

Regards

--
Julian Anastasov
--
 
 

Gesendet: Freitag, 22. August 2014 um 18:28 Uhr
Von: "Julian Anastasov" <ja@ssi.bg>
An: "Fedor Babkin" <fedor.babkin@gmx.net>
Cc: netdev@vger.kernel.org
Betreff: Re: Routes with unreachable gateways are staying in the routing table and they are functional
Hello, On Fri, 22 Aug 2014, Fedor Babkin wrote: > Hello, > > I noticed that in case a network interface has two addresses assigned (no matter if IPv4 or IPv6) from subnets A and B, the route to reach subnet C via the gateway in A is staying configured and functional when subnet A is removed from the interface. Let me illustrate the behavior with IPv4 example. > > Starting point. eth0 has 2 subnets, the route to reach 3-rd subnet via gw in one of them is present: > eth0: mtu 1500 qdisc pfifo_fast state UP group default qlen 1000 > link/ether 52:54:00:12:34:57 brd ff:ff:ff:ff:ff:ff > inet 10.20.10.3/24 brd 10.20.10.255 scope global eth0 > valid_lft forever preferred_lft forever > inet 10.30.10.3/24 brd 10.30.10.255 scope global eth0 > valid_lft forever preferred_lft forever > inet6 fe80::5054:ff:fe12:3457/64 scope link > valid_lft forever preferred_lft forever > root@ubuntu14-4-vm:~# ip ro ls > 10.11.10.0/24 via 10.30.10.1 dev eth0 > 10.20.10.0/24 dev eth0 proto kernel scope link src 10.20.10.3 > 10.30.10.0/24 dev eth0 proto kernel scope link src 10.30.10.3 > > Delete 10.30.10.3/24, the indirect route persists: > root@ubuntu14-4-vm:~# ip ro ls > 10.11.10.0/24 via 10.30.10.1 dev eth0 > 10.20.10.0/24 dev eth0 proto kernel scope link src 10.20.10.3 > > Ping to destinations in 10.11.10.0/24 works, unreachable gateway 10.30.10.1 is resolved, even recovers in case of neighbor flushes. > root@ubuntu14-4-vm:~# ping 10.11.10.1 > PING 10.11.10.1 (10.11.10.1) 56(84) bytes of data. > 64 bytes from 10.11.10.1: icmp_seq=1 ttl=64 time=0.706 ms > root@ubuntu14-4-vm:~# ip n l > 10.30.10.1 dev eth0 lladdr 52:54:00:12:34:56 REACHABLE > > The above was observed with kernel version 2.6.32 and confirmed with 3.13.0 In case of IPv6 this behavior is more disturbing as there is always LLA assigned, so applications have to orchestrate the removal of unreachable routes. > If it's an intended implementation, would be helpful to know a reason behind it. Thanks for your support. It is expensive to implement solution for this problem. If we try to be very strict by handling it at FIB level (catch route removal) simple operations like secondary address promotion or route replacement can lead to cascade of route removals. At the end, it is again the user who have to take care to add all lost routes back. So, it is a complex task to solve. Here is recent discussion on such topic: http://marc.info/?t=139030500700005&r=1&w=2 http://marc.info/?t=139055890700002&r=1&w=2 Regards -- Julian Anastasov

^ permalink raw reply

* Re: [PATCH] net: stmmac: fix warning from Sparse for socfpga
From: Ley Foon Tan @ 2014-08-26  7:47 UTC (permalink / raw)
  To: Giuseppe CAVALLARO
  Cc: netdev, linux-kernel@vger.kernel.org, David S. Miller,
	Vince Bridgers
In-Reply-To: <53FC363E.6050604@st.com>

On Tue, Aug 26, 2014 at 3:24 PM, Giuseppe CAVALLARO
<peppe.cavallaro@st.com> wrote:
>> @@ -119,7 +119,8 @@ static int socfpga_dwmac_parse_data(struct
>> socfpga_dwmac *dwmac, struct device *
>>                         return -EINVAL;
>>                 }
>>
>> -               dwmac->splitter_base = (void *)devm_ioremap_resource(dev,
>> +               dwmac->splitter_base =
>> +                       (void __iomem *)devm_ioremap_resource(dev,
>
>
> I think, no casting should be done:
>
>    dwmac->splitter_base = devm_ioremap_resource(dev, ....
Oh ya, since both are same type. Will send new patch.

>
> patch should be for net-next
Do you mean the patch need based on net-next git?
I'm using linux-next git now.

Thanks.

Regards
Ley Foon

^ permalink raw reply

* Re: [PATCH v6 net-next 4/6] bpf: enable bpf syscall on x64 and i386
From: Ingo Molnar @ 2014-08-26  7:46 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Stephen Hemminger, David S. Miller, Linus Torvalds,
	Andy Lutomirski, Steven Rostedt, Daniel Borkmann, Chema Gonzalez,
	Eric Dumazet, Peter Zijlstra, Brendan Gregg, Namhyung Kim,
	H. Peter Anvin, Andrew Morton, Kees Cook, Linux API,
	Network Development, LKML
In-Reply-To: <CAMEtUuxicvHsEQ59CeG2kmLW0b4TQ9ExfDkKo2kktH-t8x5tBg@mail.gmail.com>


* Alexei Starovoitov <ast@plumgrid.com> wrote:

> On Mon, Aug 25, 2014 at 8:52 PM, Stephen Hemminger
> <stephen@networkplumber.org> wrote:
> > Per discussion at Kernel Summit. Every new syscall requires
> > a manual page and test programs. We have had too many new syscalls
> > that are DOA.
> 
> There is verifier testsuite that is testing eBPF verifier from userspace
> via bpf syscall. Also there are multiple examples and libbpf.
> I think test coverage for bpf syscall is quite substantial already.

This is in tools/bpf/, right?

Thanks,

	Ingo

^ permalink raw reply

* Re: [PATCH v6 net-next 4/6] bpf: enable bpf syscall on x64 and i386
From: Ingo Molnar @ 2014-08-26  7:45 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: David Miller, Linus Torvalds, Andy Lutomirski, Steven Rostedt,
	Daniel Borkmann, Chema Gonzalez, Eric Dumazet, Peter Zijlstra,
	Brendan Gregg, Namhyung Kim, H. Peter Anvin, Andrew Morton,
	Kees Cook, Linux API, Network Development, LKML
In-Reply-To: <CAMEtUuy1DXFMABAg2Uup5HtqmiJHw0WR=or-z9CfpVMscrVcVg@mail.gmail.com>


* Alexei Starovoitov <ast@plumgrid.com> wrote:

> On Mon, Aug 25, 2014 at 6:07 PM, David Miller <davem@davemloft.net> wrote:
> > From: Alexei Starovoitov <ast@plumgrid.com>
> > Date: Mon, 25 Aug 2014 18:00:56 -0700
> >
> >> -
> >> +asmlinkage long sys_bpf(int cmd, unsigned long arg2, unsigned long arg3,
> >> +                     unsigned long arg4, unsigned long arg5);
> >
> > Please do not add interfaces with opaque types as arguments.
> >
> > It is impossible for the compiler to type check the args at
> > compile time when userspace tries to use this stuff.
> 
> I share this concern. I went with single BPF syscall, because
> alternative is 6 syscalls for every command and more
> syscalls in the future when we'd need to add another command.

We had a similar problem growing the perf syscall - and we were 
able to hold to a single syscall, which I think has served us 
well. Had we gone with a per functionality syscall we'd have 
something like a dozen syscalls today, scattered all around 
non-continuously in the syscall space on most platforms.

But note that 'opaque or non-opaque' is a false dichotomy, as 
there are 3 options in reality: what we used instead of an opaque 
type was an extensible data type, and extensible C structure, 
with structure size expectations part of the structure.

See 'struct perf_event_attr':

SYSCALL_DEFINE5(perf_event_open,
                struct perf_event_attr __user *, attr_uptr,
                pid_t, pid, int, cpu, int, group_fd, unsigned long, flags)

That way new versions of the data type are immediately obvious to 
the kernel, and compatibility can be handled well. Smaller, 
previous versions received from old user-space are padded out 
transparently to the kernel's value of the structure, with zeroes 
filled in.

See perf_copy_attr() in kernel/events/core.c. Instead of 
versioning the structure, we use its size as a finegrained and 
robust version indicator in essence.

That way it's both forwards and backwards compatible, as much as 
possible technically: old kernel can run new user-space, and new 
user-space will be able to take advantage of as much of an old 
kernel's capabilities as possible, and in the typical case of 
version match there's no extra overhead worth speaking of.

This way we were able to gradually grow to the sophisticated ABI 
you can find in include/uapi/linux/perf_event.h, without having 
to touch the syscall interface. (It's not the only method: we 
also have a handful of ioctls, where that's the most natural 
interface for a perf event fd.)

Thanks,

	Ingo

^ permalink raw reply

* Re: igbvf warning on 3.14.x
From: William Dauchy @ 2014-08-26  7:36 UTC (permalink / raw)
  To: Carolyn Wyborny, mitch.a.williams
  Cc: William Dauchy, netdev, e1000-devel, Jeff Kirsher
In-Reply-To: <1408109225.2391.30.camel@jtkirshe-mobl>

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

Hi,

On Aug15 06:27, Jeff Kirsher wrote:
> Adding Carolyn (igb maintainer) and e1000-devel mailing list...

any chance to get some feedback on this trace?

Thanks,
-- 
William

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply

* Re: [PATCH net-next v2 2/2] xfrm: configure policy hash table thresholds by netlink
From: Christophe Gouault @ 2014-08-26  7:27 UTC (permalink / raw)
  To: Steffen Klassert; +Cc: David S. Miller, netdev@vger.kernel.org
In-Reply-To: <20140821060944.GC6390@secunet.com>

2014-08-21 8:09 GMT+02:00 Steffen Klassert <steffen.klassert@secunet.com>:
> On Fri, Aug 01, 2014 at 11:12:28AM +0200, Christophe Gouault wrote:
>> diff --git a/include/net/netns/xfrm.h b/include/net/netns/xfrm.h
>> index 41902a8..9da7982 100644
>> --- a/include/net/netns/xfrm.h
>> +++ b/include/net/netns/xfrm.h
>> @@ -19,6 +19,15 @@ struct xfrm_policy_hash {
>>       u8                      sbits6;
>>  };
>>
>> +struct xfrm_policy_hthresh {
>> +     struct work_struct      work;
>> +     seqlock_t               lock;
>
> This newly introduced lock is not initialized. It triggers an
> inconsistent lock state warning when acquired for the first time.

oops! I'll fix that.

>> +     pr_info("rebuilding SPD hash table: thresholds (%u,%u)(%u,%u)\n",
>> +             lbits4, rbits4, lbits6, rbits6);
>
> Do we really need to print this?

No, it's not necessary, I will remove it.

>> +             hlist_for_each_entry(pol, chain, bydst) {
>> +                     if (policy->priority >= pol->priority)
>> +                             newpos = &pol->bydst;
>> +                     else
>> +                             break;
>> +             }
>> +             if (newpos)
>> +                     hlist_add_after(newpos, &policy->bydst);
>
> hlist_add_after() does not exist any more, it was replaced by
> hlist_add_behind() recently.

OK, I'll update the code accordingly.

>> +static int xfrm_set_spdinfo(struct sk_buff *skb, struct nlmsghdr *nlh,
>> +                         struct nlattr **attrs)
>> +{
>> +     struct net *net = sock_net(skb->sk);
>> +     struct sk_buff *r_skb;
>> +     u32 *flags = nlmsg_data(nlh);
>> +     u32 sportid = NETLINK_CB(skb).portid;
>> +     u32 seq = nlh->nlmsg_seq;
>> +     struct xfrmu_spdhthresh *thresh4 = NULL;
>> +     struct xfrmu_spdhthresh *thresh6 = NULL;
>> +
>> +     /* selector prefixlen thresholds to hash policies */
>> +     if (attrs[XFRMA_SPD_IPV4_HTHRESH]) {
>> +             struct nlattr *rta = attrs[XFRMA_SPD_IPV4_HTHRESH];
>> +
>> +             if (nla_len(rta) < sizeof(*thresh4))
>> +                     return -EINVAL;
>> +             thresh4 = nla_data(rta);
>> +             if (thresh4->lbits > 32 || thresh4->rbits > 32)
>> +                     return -EINVAL;
>> +     }
>> +     if (attrs[XFRMA_SPD_IPV6_HTHRESH]) {
>> +             struct nlattr *rta = attrs[XFRMA_SPD_IPV6_HTHRESH];
>> +
>> +             if (nla_len(rta) < sizeof(*thresh6))
>> +                     return -EINVAL;
>> +             thresh6 = nla_data(rta);
>> +             if (thresh6->lbits > 128 || thresh6->rbits > 128)
>> +                     return -EINVAL;
>> +     }
>> +
>> +     if (thresh4 || thresh6) {
>> +             write_seqlock(&net->xfrm.policy_hthresh.lock);
>> +             if (thresh4) {
>> +                     net->xfrm.policy_hthresh.lbits4 = thresh4->lbits;
>> +                     net->xfrm.policy_hthresh.rbits4 = thresh4->rbits;
>> +             }
>> +             if (thresh6) {
>> +                     net->xfrm.policy_hthresh.lbits6 = thresh6->lbits;
>> +                     net->xfrm.policy_hthresh.rbits6 = thresh6->rbits;
>> +             }
>> +             write_sequnlock(&net->xfrm.policy_hthresh.lock);
>> +
>> +             xfrm_policy_hash_rebuild(net);
>> +     }
>> +
>> +     r_skb = nlmsg_new(xfrm_spdinfo_msgsize(), GFP_ATOMIC);
>> +     if (r_skb == NULL)
>> +             return -ENOMEM;
>> +
>> +     if (build_spdinfo(r_skb, net, sportid, seq, *flags) < 0)
>> +             BUG();
>> +
>> +     return nlmsg_unicast(net->xfrm.nlsk, r_skb, sportid);
>
> Why do you send these informations to userspace? This is a set
> operation, not get.

You're right, I'll remove this reply message.

> The rest looks quite good, thanks!

Thanks. I'll send an update.

Christophe

^ permalink raw reply

* [PATCH (net.git)] phy: fix EEE checks inside the phy_init_eee.
From: Giuseppe Cavallaro @ 2014-08-26  7:26 UTC (permalink / raw)
  To: netdev; +Cc: Giuseppe Cavallaro, Nandini Sharma

According to the Std 802.3az if the EEE Adv (Reg 7.60), Link partner ability
(Reg 7.61) and EEE capability (Register 3.20) bits return 0 this  means no EEE
is supported. So this patch fixes the checks inside the phy_init_eee function.

Signed-off-by: Nandini Sharma <nandini.sharma@st.com>
Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
---
 drivers/net/phy/phy.c |   18 +++++++++---------
 1 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index c94e2a2..a854d38 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -1036,31 +1036,31 @@ int phy_init_eee(struct phy_device *phydev, bool clk_stop_enable)
 		/* First check if the EEE ability is supported */
 		eee_cap = phy_read_mmd_indirect(phydev, MDIO_PCS_EEE_ABLE,
 						MDIO_MMD_PCS, phydev->addr);
-		if (eee_cap < 0)
-			return eee_cap;
+		if (eee_cap <= 0)
+			goto eee_exit_err;
 
 		cap = mmd_eee_cap_to_ethtool_sup_t(eee_cap);
 		if (!cap)
-			return -EPROTONOSUPPORT;
+			goto eee_exit_err;
 
 		/* Check which link settings negotiated and verify it in
 		 * the EEE advertising registers.
 		 */
 		eee_lp = phy_read_mmd_indirect(phydev, MDIO_AN_EEE_LPABLE,
 					       MDIO_MMD_AN, phydev->addr);
-		if (eee_lp < 0)
-			return eee_lp;
+		if (eee_lp <= 0)
+			goto eee_exit_err;
 
 		eee_adv = phy_read_mmd_indirect(phydev, MDIO_AN_EEE_ADV,
 						MDIO_MMD_AN, phydev->addr);
-		if (eee_adv < 0)
-			return eee_adv;
+		if (eee_adv <= 0)
+			goto eee_exit_err;
 
 		adv = mmd_eee_adv_to_ethtool_adv_t(eee_adv);
 		lp = mmd_eee_adv_to_ethtool_adv_t(eee_lp);
 		idx = phy_find_setting(phydev->speed, phydev->duplex);
 		if (!(lp & adv & settings[idx].setting))
-			return -EPROTONOSUPPORT;
+			goto eee_exit_err;
 
 		if (clk_stop_enable) {
 			/* Configure the PHY to stop receiving xMII
@@ -1080,7 +1080,7 @@ int phy_init_eee(struct phy_device *phydev, bool clk_stop_enable)
 
 		return 0; /* EEE supported */
 	}
-
+eee_exit_err:
 	return -EPROTONOSUPPORT;
 }
 EXPORT_SYMBOL(phy_init_eee);
-- 
1.7.4.4

^ permalink raw reply related

* [PATCH net-next] bnx2x: Fix static checker warning regarding `txdata_ptr'
From: Yuval Mintz @ 2014-08-26  7:24 UTC (permalink / raw)
  To: davem; +Cc: netdev, kernel-janitors, Ariel.Elior, dan.carpenter, Yuval Mintz

Incorrect checking of array instead of array contents in panic_dump
flow - results of commit e261199872a2 ("bnx2x: Safe bnx2x_panic_dump()").

Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Yuval Mintz <Yuval.Mintz@qlogic.com>
---
Hi Dave,

Please apply this to `net-next'.

Thanks,
Yuval Mintz
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
index 2f394b8..93132d8f 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
@@ -985,7 +985,7 @@ void bnx2x_panic_dump(struct bnx2x *bp, bool disable_int)
 		/* Tx */
 		for_each_cos_in_tx_queue(fp, cos)
 		{
-			if (!fp->txdata_ptr)
+			if (!fp->txdata_ptr[cos])
 				break;
 
 			txdata = *fp->txdata_ptr[cos];
@@ -1140,7 +1140,7 @@ void bnx2x_panic_dump(struct bnx2x *bp, bool disable_int)
 		for_each_cos_in_tx_queue(fp, cos) {
 			struct bnx2x_fp_txdata *txdata = fp->txdata_ptr[cos];
 
-			if (!fp->txdata_ptr)
+			if (!fp->txdata_ptr[cos])
 				break;
 
 			if (!txdata->tx_cons_sb)
-- 
1.8.3.1

^ permalink raw reply related

* Re: [PATCH] net: stmmac: fix warning from Sparse for socfpga
From: Giuseppe CAVALLARO @ 2014-08-26  7:24 UTC (permalink / raw)
  To: Ley Foon Tan, netdev, linux-kernel, David S. Miller
  Cc: lftan.linux, Vince Bridgers
In-Reply-To: <1409037076-14775-1-git-send-email-lftan@altera.com>

On 8/26/2014 9:11 AM, Ley Foon Tan wrote:
> Warning:
> drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c:122:41:
> sparse: cast removes address space of expression
> drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c:122:38:
> sparse: incorrect type in assignment (different address spaces)
>
> Signed-off-by: Ley Foon Tan <lftan@altera.com>
> ---
>   drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
> index cd613d7..c1addce 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
> @@ -119,7 +119,8 @@ static int socfpga_dwmac_parse_data(struct socfpga_dwmac *dwmac, struct device *
>   			return -EINVAL;
>   		}
>
> -		dwmac->splitter_base = (void *)devm_ioremap_resource(dev,
> +		dwmac->splitter_base =
> +			(void __iomem *)devm_ioremap_resource(dev,

I think, no casting should be done:

    dwmac->splitter_base = devm_ioremap_resource(dev, ....


patch should be for net-next

peppe

>   			&res_splitter);
>   		if (!dwmac->splitter_base) {
>   			dev_info(dev, "Failed to mapping emac splitter\n");
>

^ permalink raw reply

* Re: [PATCH net-next 2/2] net: exit busy loop when another process is runnable
From: Jason Wang @ 2014-08-26  7:16 UTC (permalink / raw)
  To: Eliezer Tamir, Eric Dumazet
  Cc: Ingo Molnar, Mike Galbraith, davem, netdev, linux-kernel, mst,
	Peter Zijlstra, Ingo Molnar jacob.e.keller@intel.com
In-Reply-To: <53FB3715.7070009@linux.intel.com>

On 08/25/2014 09:16 PM, Eliezer Tamir wrote:
> On 22/08/2014 17:16, Eric Dumazet wrote:
>> > On Fri, 2014-08-22 at 17:08 +0800, Jason Wang wrote:
>> > 
>>> >> But this is just for current process. We want to determine whether or
>>> >> not it was worth to loop busily in current process by checking if
>>> >> there's any another runnable processes or callbacks. And what we need
>>> >> here is just a simple and lockless hint which can't be wrong but may be
>>> >> inaccurate to exit the busy loop. The net code does not depends on this
>>> >> hint to do scheduling or yielding.
>>> >>
>>> >> How about just introducing a boolean helper like current_can_busy_loop()
>>> >> and return true in one of the following conditions:
>>> >>
>>> >> - Current task is SCHED_FIFO
>>> >> - Current task is neither SCHED_FIFO nor SCHED_IDLE and no other
>>> >> runnable processes or pending RCU callbacks in current cpu
>>> >>
>>> >> And add warns to make sure it can only be called in process context.
>> > 
>> > 
>> > 1) Any reasons Eliezer Tamir is not included in the CC list ?
> Thanks for remembering me, Eric ;)
>
> Here are my 2 cents:
> I think Ingo's suggestion of only yielding to tasks with same or higher
> priority makes sense.

I'm not sure I get your meaning. Do you mean calling yield_to() directly
in sk_busy_loop?

Schedule() which will be called later should handle all cases such as
priority and rt process. And this patch just want the schedule() to do
this decision earlier by exiting the busy loop earlier. This will
improve the latency in both heavy load and light load.

Checking number of nsecs this task is expected to run in the future
sounds like the work that sk_busy_loop_end_time() should consider. It
was not the issue that this patch want to address.
>
> IF you change the current behavior, please update the documentation.
> You are going to make people scratch their head and ask "what changed?"
> you owe them a clue.

Thanks for the reminding. But for this patch itself, it does not change
user noticeable behaviour.
> I also would like to have some way to keep track of when/if/how much
> this yield happens.
>

Ok, not very hard to add, maybe just another statistics counter.

^ permalink raw reply

* [PATCH (net.git) 4/4] stmmac: fix PLS bit setting when EEE is active
From: Giuseppe Cavallaro @ 2014-08-26  7:16 UTC (permalink / raw)
  To: netdev; +Cc: Giuseppe Cavallaro, nandini sharma
In-Reply-To: <1409037383-3213-1-git-send-email-peppe.cavallaro@st.com>

In case of PLS is active the PLS (PHY Link Status) bit in
the Reg12 has to be set to allow the MAC to asserts the LPI
pattern when the link is ok.

Signed-off-by: nandini sharma <nandini.sharma@st.com>
Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 48a112f..5b5bbc3 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -321,10 +321,9 @@ bool stmmac_eee_init(struct stmmac_priv *priv)
 			priv->hw->mac->set_eee_timer(priv->hw,
 						     STMMAC_DEFAULT_LIT_LS,
 						     tx_lpi_timer);
-		} else
-			/* Set HW EEE according to the speed */
-			priv->hw->mac->set_eee_pls(priv->hw,
-						   priv->phydev->link);
+		}
+		/* Set HW EEE according to the speed */
+		priv->hw->mac->set_eee_pls(priv->hw, priv->phydev->link);
 
 		pr_debug("stmmac: Energy-Efficient Ethernet initialized\n");
 
-- 
1.7.4.4

^ permalink raw reply related

* [PATCH (net.git) 3/4] stmmac: never check EEE in case of a switch is attached
From: Giuseppe Cavallaro @ 2014-08-26  7:16 UTC (permalink / raw)
  To: netdev; +Cc: Giuseppe CAVALLARO
In-Reply-To: <1409037383-3213-1-git-send-email-peppe.cavallaro@st.com>

From: Giuseppe CAVALLARO <peppe.cavallaro@st.com>

This patch is to skip the EEE initialisation when the stmmac
is using a switch (with a fixed phy support).

Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 51a89d4..48a112f 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -276,6 +276,7 @@ static void stmmac_eee_ctrl_timer(unsigned long arg)
 bool stmmac_eee_init(struct stmmac_priv *priv)
 {
 	bool ret = false;
+	char *phy_bus_name = priv->plat->phy_bus_name;
 
 	/* Using PCS we cannot dial with the phy registers at this stage
 	 * so we do not support extra feature like EEE.
@@ -284,6 +285,10 @@ bool stmmac_eee_init(struct stmmac_priv *priv)
 	    (priv->pcs == STMMAC_PCS_RTBI))
 		goto out;
 
+	/* Never init EEE in case of a switch is attached */
+	if (phy_bus_name && (!strcmp(phy_bus_name, "fixed")))
+		goto out;
+
 	/* MAC core supports the EEE feature. */
 	if (priv->dma_cap.eee) {
 		int tx_lpi_timer = priv->tx_lpi_timer;
-- 
1.7.4.4

^ permalink raw reply related

* [PATCH (net.git) 2/4] stmmac: fix LPI TW timer value to 20.5us.
From: Giuseppe Cavallaro @ 2014-08-26  7:16 UTC (permalink / raw)
  To: netdev; +Cc: nandini sharma, Giuseppe Cavallaro
In-Reply-To: <1409037383-3213-1-git-send-email-peppe.cavallaro@st.com>

From: nandini sharma <nandini.sharma@st.com>

The value for LPI TW timer has to be updated to 0x1E that is the hardcoded value
of 20.5us and it will apply to all EEE enabled Remote PHYs.
Disadvantage is for PHY's that support lesser wakeup time but we can accept it
waiting to implement LLDP to negotiate the Wakeup time of Remote PHY.

Signed-off-by: nandini sharma <nandini.sharma@st.com>
Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
---
 drivers/net/ethernet/stmicro/stmmac/common.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
index a464e8c..9f3e8b4 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
@@ -287,7 +287,7 @@ struct dma_features {
 
 /* Default LPI timers */
 #define STMMAC_DEFAULT_LIT_LS	0x3E8
-#define STMMAC_DEFAULT_TWT_LS	0x0
+#define STMMAC_DEFAULT_TWT_LS	0x1E
 
 #define STMMAC_CHAIN_MODE	0x1
 #define STMMAC_RING_MODE	0x2
-- 
1.7.4.4

^ permalink raw reply related

* [PATCH (net.git) 1/4] stmmac: fix the EEE LPI Macro definitions.
From: Giuseppe Cavallaro @ 2014-08-26  7:16 UTC (permalink / raw)
  To: netdev; +Cc: nandini sharma, Giuseppe Cavallaro
In-Reply-To: <1409037383-3213-1-git-send-email-peppe.cavallaro@st.com>

From: nandini sharma <nandini.sharma@st.com>

This patch is to fix the definition of macros for EEE otherwise the LPI TX/RX
entry/exit cannot be properly managed.

Signed-off-by: Nandini Sharma <nandini.sharma@st.com>
Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
---
 drivers/net/ethernet/stmicro/stmmac/common.h |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
index de507c3..a464e8c 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
@@ -220,10 +220,10 @@ enum dma_irq_status {
 	handle_tx = 0x8,
 };
 
-#define	CORE_IRQ_TX_PATH_IN_LPI_MODE	(1 << 1)
-#define	CORE_IRQ_TX_PATH_EXIT_LPI_MODE	(1 << 2)
-#define	CORE_IRQ_RX_PATH_IN_LPI_MODE	(1 << 3)
-#define	CORE_IRQ_RX_PATH_EXIT_LPI_MODE	(1 << 4)
+#define	CORE_IRQ_TX_PATH_IN_LPI_MODE	(1 << 0)
+#define	CORE_IRQ_TX_PATH_EXIT_LPI_MODE	(1 << 1)
+#define	CORE_IRQ_RX_PATH_IN_LPI_MODE	(1 << 2)
+#define	CORE_IRQ_RX_PATH_EXIT_LPI_MODE	(1 << 3)
 
 #define	CORE_PCS_ANE_COMPLETE		(1 << 5)
 #define	CORE_PCS_LINK_STATUS		(1 << 6)
-- 
1.7.4.4

^ permalink raw reply related

* [PATCH (net.git) 0/4] stmmac EEE fixes
From: Giuseppe Cavallaro @ 2014-08-26  7:16 UTC (permalink / raw)
  To: netdev; +Cc: Giuseppe Cavallaro

This is a subset of patches to provide some fixes for the EEE support inside the
driver.
Patches have been tested on boards EEE capable plugged on switch w/ w/o EEE
support.

Giuseppe CAVALLARO (1):
  stmmac: never check EEE in case of a switch is attached

Giuseppe Cavallaro (1):
  stmmac: fix PLS bit setting when EEE is active

nandini sharma (2):
  stmmac: fix the EEE LPI Macro definitions.
  stmmac: fix LPI TW timer value to 20.5us.

 drivers/net/ethernet/stmicro/stmmac/common.h      |   10 +++++-----
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c |   12 ++++++++----
 2 files changed, 13 insertions(+), 9 deletions(-)

-- 
1.7.4.4

^ permalink raw reply

* [PATCH] net: stmmac: fix warning from Sparse for socfpga
From: Ley Foon Tan @ 2014-08-26  7:11 UTC (permalink / raw)
  To: netdev, linux-kernel, David S. Miller
  Cc: Ley Foon Tan, lftan.linux, Giuseppe Cavallaro, Vince Bridgers

Warning:
drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c:122:41:
sparse: cast removes address space of expression
drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c:122:38:
sparse: incorrect type in assignment (different address spaces)

Signed-off-by: Ley Foon Tan <lftan@altera.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
index cd613d7..c1addce 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
@@ -119,7 +119,8 @@ static int socfpga_dwmac_parse_data(struct socfpga_dwmac *dwmac, struct device *
 			return -EINVAL;
 		}
 
-		dwmac->splitter_base = (void *)devm_ioremap_resource(dev,
+		dwmac->splitter_base =
+			(void __iomem *)devm_ioremap_resource(dev,
 			&res_splitter);
 		if (!dwmac->splitter_base) {
 			dev_info(dev, "Failed to mapping emac splitter\n");
-- 
1.8.2.1

^ 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