* [PATCH net-next 03/11] tipc: remove retransmission queue
From: Ying Xue @ 2014-11-26 3:41 UTC (permalink / raw)
To: davem; +Cc: jon.maloy, netdev, Paul.Gortmaker, tipc-discussion
In-Reply-To: <1416973315-6047-1-git-send-email-ying.xue@windriver.com>
TIPC retransmission queue is intended to record which messages
should be retransmitted when bearer is not congested. However,
as the retransmission queue becomes useless with the removal of
bearer congestion mechanism, it should be removed.
Signed-off-by: Ying Xue <ying.xue@windriver.com>
Reviewed-by: Jon Maloy <jon.maloy@ericsson.com>
---
net/tipc/link.c | 35 +----------------------------------
net/tipc/link.h | 4 ----
2 files changed, 1 insertion(+), 38 deletions(-)
diff --git a/net/tipc/link.c b/net/tipc/link.c
index 450ed0c..4b7cbfd 100644
--- a/net/tipc/link.c
+++ b/net/tipc/link.c
@@ -458,8 +458,6 @@ void tipc_link_reset(struct tipc_link *l_ptr)
skb_queue_splice_init(&l_ptr->waiting_sks, &owner->waiting_sks);
owner->action_flags |= TIPC_WAKEUP_USERS;
}
- l_ptr->retransm_queue_head = 0;
- l_ptr->retransm_queue_size = 0;
l_ptr->last_out = NULL;
l_ptr->first_out = NULL;
l_ptr->next_out = NULL;
@@ -870,38 +868,9 @@ static void tipc_link_sync_rcv(struct tipc_node *n, struct sk_buff *buf)
*/
static u32 tipc_link_push_packet(struct tipc_link *l_ptr)
{
- struct sk_buff *buf = l_ptr->first_out;
- u32 r_q_size = l_ptr->retransm_queue_size;
- u32 r_q_head = l_ptr->retransm_queue_head;
-
- /* Step to position where retransmission failed, if any, */
- /* consider that buffers may have been released in meantime */
- if (r_q_size && buf) {
- u32 last = lesser(mod(r_q_head + r_q_size),
- link_last_sent(l_ptr));
- u32 first = buf_seqno(buf);
-
- while (buf && less(first, r_q_head)) {
- first = mod(first + 1);
- buf = buf->next;
- }
- l_ptr->retransm_queue_head = r_q_head = first;
- l_ptr->retransm_queue_size = r_q_size = mod(last - first);
- }
-
- /* Continue retransmission now, if there is anything: */
- if (r_q_size && buf) {
- msg_set_ack(buf_msg(buf), mod(l_ptr->next_in_no - 1));
- msg_set_bcast_ack(buf_msg(buf), l_ptr->owner->bclink.last_in);
- tipc_bearer_send(l_ptr->bearer_id, buf, &l_ptr->media_addr);
- l_ptr->retransm_queue_head = mod(++r_q_head);
- l_ptr->retransm_queue_size = --r_q_size;
- l_ptr->stats.retransmitted++;
- return 0;
- }
+ struct sk_buff *buf = l_ptr->next_out;
/* Send one deferred data message, if send window not full: */
- buf = l_ptr->next_out;
if (buf) {
struct tipc_msg *msg = buf_msg(buf);
u32 next = msg_seqno(msg);
@@ -1025,8 +994,6 @@ void tipc_link_retransmit(struct tipc_link *l_ptr, struct sk_buff *buf,
retransmits--;
l_ptr->stats.retransmitted++;
}
-
- l_ptr->retransm_queue_head = l_ptr->retransm_queue_size = 0;
}
/**
diff --git a/net/tipc/link.h b/net/tipc/link.h
index fb3f99b..cc816aa 100644
--- a/net/tipc/link.h
+++ b/net/tipc/link.h
@@ -130,8 +130,6 @@ struct tipc_stats {
* @oldest_deferred_in: ptr to first inbound message in queue
* @newest_deferred_in: ptr to last inbound message in queue
* @unacked_window: # of inbound messages rx'd without ack'ing back to peer
- * @retransm_queue_size: number of messages to retransmit
- * @retransm_queue_head: sequence number of first message to retransmit
* @next_out: ptr to first unsent outbound message in queue
* @waiting_sks: linked list of sockets waiting for link congestion to abate
* @long_msg_seq_no: next identifier to use for outbound fragmented messages
@@ -190,8 +188,6 @@ struct tipc_link {
u32 unacked_window;
/* Congestion handling */
- u32 retransm_queue_size;
- u32 retransm_queue_head;
struct sk_buff *next_out;
struct sk_buff_head waiting_sks;
--
1.7.9.5
------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=157005751&iu=/4140/ostg.clktrk
^ permalink raw reply related
* [PATCH net-next 01/11] tipc: remove node subscription infrastructure
From: Ying Xue @ 2014-11-26 3:41 UTC (permalink / raw)
To: davem; +Cc: jon.maloy, netdev, Paul.Gortmaker, tipc-discussion
In-Reply-To: <1416973315-6047-1-git-send-email-ying.xue@windriver.com>
The node subscribe infrastructure represents a virtual base class, so
its users, such as struct tipc_port and struct publication, can derive
its implemented functionalities. However, after the removal of struct
tipc_port, struct publication is left as its only single user now. So
defining an abstract infrastructure for one user becomes no longer
reasonable. If corresponding new functions associated with the
infrastructure are moved to name_table.c file, the node subscription
infrastructure can be removed as well.
Signed-off-by: Ying Xue <ying.xue@windriver.com>
Reviewed-by: Jon Maloy <jon.maloy@ericsson.com>
---
net/tipc/Makefile | 4 +-
net/tipc/name_distr.c | 52 ++++++++++++++++++++++----
net/tipc/name_distr.h | 1 +
net/tipc/name_table.c | 2 +-
net/tipc/name_table.h | 6 +--
net/tipc/node.c | 6 +--
net/tipc/node.h | 5 +--
net/tipc/node_subscr.c | 96 ------------------------------------------------
net/tipc/node_subscr.h | 63 -------------------------------
9 files changed, 56 insertions(+), 179 deletions(-)
delete mode 100644 net/tipc/node_subscr.c
delete mode 100644 net/tipc/node_subscr.h
diff --git a/net/tipc/Makefile b/net/tipc/Makefile
index b8a13ca..333e459 100644
--- a/net/tipc/Makefile
+++ b/net/tipc/Makefile
@@ -7,8 +7,8 @@ obj-$(CONFIG_TIPC) := tipc.o
tipc-y += addr.o bcast.o bearer.o config.o \
core.o link.o discover.o msg.o \
name_distr.o subscr.o name_table.o net.o \
- netlink.o node.o node_subscr.o \
- socket.o log.o eth_media.o server.o
+ netlink.o node.o socket.o log.o eth_media.o \
+ server.o
tipc-$(CONFIG_TIPC_MEDIA_IB) += ib_media.o
tipc-$(CONFIG_SYSCTL) += sysctl.o
diff --git a/net/tipc/name_distr.c b/net/tipc/name_distr.c
index 376d2bb..6c2638d 100644
--- a/net/tipc/name_distr.c
+++ b/net/tipc/name_distr.c
@@ -250,13 +250,45 @@ void tipc_named_node_up(u32 dnode)
tipc_link_xmit(buf_chain, dnode, dnode);
}
+static void tipc_publ_subscribe(struct publication *publ, u32 addr)
+{
+ struct tipc_node *node;
+
+ if (in_own_node(addr))
+ return;
+
+ node = tipc_node_find(addr);
+ if (!node) {
+ pr_warn("Node subscription rejected, unknown node 0x%x\n",
+ addr);
+ return;
+ }
+
+ tipc_node_lock(node);
+ list_add_tail(&publ->nodesub_list, &node->publ_list);
+ tipc_node_unlock(node);
+}
+
+static void tipc_publ_unsubscribe(struct publication *publ, u32 addr)
+{
+ struct tipc_node *node;
+
+ node = tipc_node_find(addr);
+ if (!node)
+ return;
+
+ tipc_node_lock(node);
+ list_del_init(&publ->nodesub_list);
+ tipc_node_unlock(node);
+}
+
/**
- * named_purge_publ - remove publication associated with a failed node
+ * tipc_publ_purge - remove publication associated with a failed node
*
* Invoked for each publication issued by a newly failed node.
* Removes publication structure from name table & deletes it.
*/
-static void named_purge_publ(struct publication *publ)
+static void tipc_publ_purge(struct publication *publ, u32 addr)
{
struct publication *p;
@@ -264,7 +296,7 @@ static void named_purge_publ(struct publication *publ)
p = tipc_nametbl_remove_publ(publ->type, publ->lower,
publ->node, publ->ref, publ->key);
if (p)
- tipc_nodesub_unsubscribe(&p->subscr);
+ tipc_publ_unsubscribe(p, addr);
write_unlock_bh(&tipc_nametbl_lock);
if (p != publ) {
@@ -277,6 +309,14 @@ static void named_purge_publ(struct publication *publ)
kfree(p);
}
+void tipc_publ_notify(struct list_head *nsub_list, u32 addr)
+{
+ struct publication *publ, *tmp;
+
+ list_for_each_entry_safe(publ, tmp, nsub_list, nodesub_list)
+ tipc_publ_purge(publ, addr);
+}
+
/**
* tipc_update_nametbl - try to process a nametable update and notify
* subscribers
@@ -294,9 +334,7 @@ static bool tipc_update_nametbl(struct distr_item *i, u32 node, u32 dtype)
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);
+ tipc_publ_subscribe(publ, node);
return true;
}
} else if (dtype == WITHDRAWAL) {
@@ -304,7 +342,7 @@ static bool tipc_update_nametbl(struct distr_item *i, u32 node, u32 dtype)
node, ntohl(i->ref),
ntohl(i->key));
if (publ) {
- tipc_nodesub_unsubscribe(&publ->subscr);
+ tipc_publ_unsubscribe(publ, node);
kfree(publ);
return true;
}
diff --git a/net/tipc/name_distr.h b/net/tipc/name_distr.h
index b9e75fe..cef55ce 100644
--- a/net/tipc/name_distr.h
+++ b/net/tipc/name_distr.h
@@ -74,5 +74,6 @@ 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);
+void tipc_publ_notify(struct list_head *nsub_list, u32 addr);
#endif
diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c
index 7cfb7a4..772be1c 100644
--- a/net/tipc/name_table.c
+++ b/net/tipc/name_table.c
@@ -144,7 +144,7 @@ static struct publication *publ_create(u32 type, u32 lower, u32 upper,
publ->key = key;
INIT_LIST_HEAD(&publ->local_list);
INIT_LIST_HEAD(&publ->pport_list);
- INIT_LIST_HEAD(&publ->subscr.nodesub_list);
+ INIT_LIST_HEAD(&publ->nodesub_list);
return publ;
}
diff --git a/net/tipc/name_table.h b/net/tipc/name_table.h
index b38ebec..c628778 100644
--- a/net/tipc/name_table.h
+++ b/net/tipc/name_table.h
@@ -37,8 +37,6 @@
#ifndef _TIPC_NAME_TABLE_H
#define _TIPC_NAME_TABLE_H
-#include "node_subscr.h"
-
struct tipc_subscription;
struct tipc_port_list;
@@ -56,7 +54,7 @@ struct tipc_port_list;
* @node: network address of publishing port's node
* @ref: publishing port
* @key: publication key
- * @subscr: subscription to "node down" event (for off-node publications only)
+ * @nodesub_list: subscription to "node down" event (off-node publication only)
* @local_list: adjacent entries in list of publications made by this node
* @pport_list: adjacent entries in list of publications made by this port
* @node_list: adjacent matching name seq publications with >= node scope
@@ -73,7 +71,7 @@ struct publication {
u32 node;
u32 ref;
u32 key;
- struct tipc_node_subscr subscr;
+ struct list_head nodesub_list;
struct list_head local_list;
struct list_head pport_list;
struct list_head node_list;
diff --git a/net/tipc/node.c b/net/tipc/node.c
index 82e5edd..17b8092 100644
--- a/net/tipc/node.c
+++ b/net/tipc/node.c
@@ -113,7 +113,7 @@ struct tipc_node *tipc_node_create(u32 addr)
spin_lock_init(&n_ptr->lock);
INIT_HLIST_NODE(&n_ptr->hash);
INIT_LIST_HEAD(&n_ptr->list);
- INIT_LIST_HEAD(&n_ptr->nsub);
+ INIT_LIST_HEAD(&n_ptr->publ_list);
INIT_LIST_HEAD(&n_ptr->conn_sks);
__skb_queue_head_init(&n_ptr->waiting_sks);
@@ -574,7 +574,7 @@ void tipc_node_unlock(struct tipc_node *node)
skb_queue_splice_init(&node->waiting_sks, &waiting_sks);
if (flags & TIPC_NOTIFY_NODE_DOWN) {
- list_replace_init(&node->nsub, &nsub_list);
+ list_replace_init(&node->publ_list, &nsub_list);
list_replace_init(&node->conn_sks, &conn_sks);
}
node->action_flags &= ~(TIPC_WAKEUP_USERS | TIPC_NOTIFY_NODE_DOWN |
@@ -591,7 +591,7 @@ void tipc_node_unlock(struct tipc_node *node)
tipc_node_abort_sock_conns(&conn_sks);
if (!list_empty(&nsub_list))
- tipc_nodesub_notify(&nsub_list);
+ tipc_publ_notify(&nsub_list, addr);
if (flags & TIPC_WAKEUP_BCAST_USERS)
tipc_bclink_wakeup_users();
diff --git a/net/tipc/node.h b/net/tipc/node.h
index 005fbce..f199451 100644
--- a/net/tipc/node.h
+++ b/net/tipc/node.h
@@ -37,7 +37,6 @@
#ifndef _TIPC_NODE_H
#define _TIPC_NODE_H
-#include "node_subscr.h"
#include "addr.h"
#include "net.h"
#include "bearer.h"
@@ -104,7 +103,7 @@ struct tipc_node_bclink {
* @link_cnt: number of links to node
* @signature: node instance identifier
* @link_id: local and remote bearer ids of changing link, if any
- * @nsub: list of "node down" subscriptions monitoring node
+ * @publ_list: list of publications
* @rcu: rcu struct for tipc_node
*/
struct tipc_node {
@@ -121,7 +120,7 @@ struct tipc_node {
int working_links;
u32 signature;
u32 link_id;
- struct list_head nsub;
+ struct list_head publ_list;
struct sk_buff_head waiting_sks;
struct list_head conn_sks;
struct rcu_head rcu;
diff --git a/net/tipc/node_subscr.c b/net/tipc/node_subscr.c
deleted file mode 100644
index 2d13eea..0000000
--- a/net/tipc/node_subscr.c
+++ /dev/null
@@ -1,96 +0,0 @@
-/*
- * net/tipc/node_subscr.c: TIPC "node down" subscription handling
- *
- * Copyright (c) 1995-2006, Ericsson AB
- * Copyright (c) 2005, 2010-2011, Wind River Systems
- * All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions are met:
- *
- * 1. Redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- * notice, this list of conditions and the following disclaimer in the
- * documentation and/or other materials provided with the distribution.
- * 3. Neither the names of the copyright holders nor the names of its
- * contributors may be used to endorse or promote products derived from
- * this software without specific prior written permission.
- *
- * Alternatively, this software may be distributed under the terms of the
- * GNU General Public License ("GPL") version 2 as published by the Free
- * Software Foundation.
- *
- * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
- * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
- * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
- * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
- * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
- * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
- * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
- * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
- * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
- * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
- * POSSIBILITY OF SUCH DAMAGE.
- */
-
-#include "core.h"
-#include "node_subscr.h"
-#include "node.h"
-
-/**
- * tipc_nodesub_subscribe - create "node down" subscription for specified node
- */
-void tipc_nodesub_subscribe(struct tipc_node_subscr *node_sub, u32 addr,
- void *usr_handle, net_ev_handler handle_down)
-{
- if (in_own_node(addr)) {
- node_sub->node = NULL;
- return;
- }
-
- node_sub->node = tipc_node_find(addr);
- if (!node_sub->node) {
- pr_warn("Node subscription rejected, unknown node 0x%x\n",
- addr);
- return;
- }
- node_sub->handle_node_down = handle_down;
- node_sub->usr_handle = usr_handle;
-
- tipc_node_lock(node_sub->node);
- list_add_tail(&node_sub->nodesub_list, &node_sub->node->nsub);
- tipc_node_unlock(node_sub->node);
-}
-
-/**
- * tipc_nodesub_unsubscribe - cancel "node down" subscription (if any)
- */
-void tipc_nodesub_unsubscribe(struct tipc_node_subscr *node_sub)
-{
- if (!node_sub->node)
- return;
-
- tipc_node_lock(node_sub->node);
- list_del_init(&node_sub->nodesub_list);
- tipc_node_unlock(node_sub->node);
-}
-
-/**
- * tipc_nodesub_notify - notify subscribers that a node is unreachable
- *
- * Note: node is locked by caller
- */
-void tipc_nodesub_notify(struct list_head *nsub_list)
-{
- struct tipc_node_subscr *ns, *safe;
- net_ev_handler handle_node_down;
-
- list_for_each_entry_safe(ns, safe, nsub_list, nodesub_list) {
- handle_node_down = ns->handle_node_down;
- if (handle_node_down) {
- ns->handle_node_down = NULL;
- handle_node_down(ns->usr_handle);
- }
- }
-}
diff --git a/net/tipc/node_subscr.h b/net/tipc/node_subscr.h
deleted file mode 100644
index d91b8cc..0000000
--- a/net/tipc/node_subscr.h
+++ /dev/null
@@ -1,63 +0,0 @@
-/*
- * net/tipc/node_subscr.h: Include file for TIPC "node down" subscription handling
- *
- * Copyright (c) 1995-2006, Ericsson AB
- * Copyright (c) 2005, 2010-2011, Wind River Systems
- * All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions are met:
- *
- * 1. Redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- * notice, this list of conditions and the following disclaimer in the
- * documentation and/or other materials provided with the distribution.
- * 3. Neither the names of the copyright holders nor the names of its
- * contributors may be used to endorse or promote products derived from
- * this software without specific prior written permission.
- *
- * Alternatively, this software may be distributed under the terms of the
- * GNU General Public License ("GPL") version 2 as published by the Free
- * Software Foundation.
- *
- * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
- * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
- * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
- * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
- * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
- * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
- * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
- * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
- * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
- * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
- * POSSIBILITY OF SUCH DAMAGE.
- */
-
-#ifndef _TIPC_NODE_SUBSCR_H
-#define _TIPC_NODE_SUBSCR_H
-
-#include "addr.h"
-
-typedef void (*net_ev_handler) (void *usr_handle);
-
-/**
- * struct tipc_node_subscr - "node down" subscription entry
- * @node: ptr to node structure of interest (or NULL, if none)
- * @handle_node_down: routine to invoke when node fails
- * @usr_handle: argument to pass to routine when node fails
- * @nodesub_list: adjacent entries in list of subscriptions for the node
- */
-struct tipc_node_subscr {
- struct tipc_node *node;
- net_ev_handler handle_node_down;
- void *usr_handle;
- struct list_head nodesub_list;
-};
-
-void tipc_nodesub_subscribe(struct tipc_node_subscr *node_sub, u32 addr,
- void *usr_handle, net_ev_handler handle_down);
-void tipc_nodesub_unsubscribe(struct tipc_node_subscr *node_sub);
-void tipc_nodesub_notify(struct list_head *nsub_list);
-
-#endif
--
1.7.9.5
------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=157005751&iu=/4140/ostg.clktrk
^ permalink raw reply related
* Re: [PATCH net 0/2] net: dsa: bcm_sf2: misc bugfixes
From: David Miller @ 2014-11-26 3:39 UTC (permalink / raw)
To: f.fainelli; +Cc: netdev
In-Reply-To: <1416967711-32494-1-git-send-email-f.fainelli@gmail.com>
From: Florian Fainelli <f.fainelli@gmail.com>
Date: Tue, 25 Nov 2014 18:08:29 -0800
> This patch series contains two bug fixes:
>
> - first patch fixes an issue on the error path of the driver where we could
> have left some of our registers mapped
>
> - second patch enforces the use of a software reset of the switch to guarantee
> the HW is in a consistent state prior to software initialization
Series applied, thanks Florian.
^ permalink raw reply
* Re: [patch net-next v3 04/17] net: introduce generic switch devices support
From: Jamal Hadi Salim @ 2014-11-26 3:33 UTC (permalink / raw)
To: Thomas Graf
Cc: Jiri Pirko, netdev, davem, nhorman, andy, dborkman, ogerlitz,
jesse, pshelar, azhou, ben, stephen, jeffrey.t.kirsher, vyasevic,
xiyou.wangcong, john.r.fastabend, edumazet, sfeldma, f.fainelli,
roopa, linville, jasowang, ebiederm, nicolas.dichtel,
ryazanov.s.a, buytenh, aviadr, nbd, alexei.starovoitov,
Neil.Jerram, ronye, simon.horman, alexander.h.duyck, john.ronciak,
mleitner, shrijeet, gospo, bcrl
In-Reply-To: <20141125215402.GA3912@casper.infradead.org>
On 11/25/14 16:54, Thomas Graf wrote:
> On 11/25/14 at 12:08pm, Jamal Hadi Salim wrote:
> It would definitely help if you could expose some more details on the
> "some network processor" you have. We're all very eager ;-)
>
Well, this thing doesnt run ovs ;-> (/me runs). If you come
to netdev i may let you play with it ;-> Its a humongous device
(think multi 100G ports).
On a serious note: Even if you took what Simon/Netronome has
(yes, I know they use ovs;->) - there is really no need for a switch
abstraction *at all* if all you want to is hang a packet
processing graph that ingresses at a port and egress at another port.
As you know, Linux supports it just fine with tc.
> I'm with Jiri but I agree it's not a perfect fit. I doubt there is but
> if you can come up with something that fits better I'm open to it.
>
> I considered "dataplane" or "dp" for a bit but it's quite generic as
> well.
>
The purpose is to offload. I think any name would be better than
mapping it to a specific abstraction called "switch". Especially
if it is hanging off a port and there is no switch in the pipeline.
cheers,
jamal
^ permalink raw reply
* Re: [patch net-next v3 09/17] bridge: add API to notify bridge driver of learned FBD on offloaded device
From: Jamal Hadi Salim @ 2014-11-26 3:22 UTC (permalink / raw)
To: Florian Fainelli, Jiri Pirko, netdev
Cc: davem, nhorman, andy, tgraf, dborkman, ogerlitz, jesse, pshelar,
azhou, ben, stephen, jeffrey.t.kirsher, vyasevic, xiyou.wangcong,
john.r.fastabend, edumazet, sfeldma, roopa, linville, jasowang,
ebiederm, nicolas.dichtel, ryazanov.s.a, buytenh, aviadr, nbd,
alexei.starovoitov, Neil.Jerram, ronye, simon.horman,
alexander.h.duyck, john.ronciak, mleitner, shrijeet, gospo, bcrl
In-Reply-To: <54750669.4090406@gmail.com>
On 11/25/14 17:44, Florian Fainelli wrote:
> On 25/11/14 02:28, Jiri Pirko wrote:
>> @@ -101,6 +101,7 @@ struct net_bridge_fdb_entry
>> unsigned char is_local;
>> unsigned char is_static;
>> unsigned char added_by_user;
>> + unsigned char added_by_external_learn;
>
> Pheww, we could be saving some memory footprint here by using different
> types here ;)
I tried to bring up this issue earlier.
Unfortuately about 15 different things being transfered over bridge
netlink use 8 bits for representing a bit of information.
A bitmap selector would be a lot more efficient. Not unusual to have a
few hundred thousand entries.
cheers,
jamal
> --
> Florian
>
^ permalink raw reply
* Re: [patch net-next v3 02/17] net: make vid as a parameter for ndo_fdb_add/ndo_fdb_del
From: Jamal Hadi Salim @ 2014-11-26 3:19 UTC (permalink / raw)
To: Scott Feldman
Cc: John Fastabend, Jiri Pirko, Netdev, David S. Miller,
nhorman@tuxdriver.com, Andy Gospodarek, Thomas Graf,
dborkman@redhat.com, ogerlitz@mellanox.com, jesse@nicira.com,
pshelar@nicira.com, azhou@nicira.com, ben@decadent.org.uk,
stephen@networkplumber.org, Kirsher, Jeffrey T,
vyasevic@redhat.com, Cong Wang, Eric Dumazet, Florian Fainelli,
Roopa Prabhu, John Linville
In-Reply-To: <CAE4R7bD1Hi+fnKp-Sg6Tn4AcpOu2pwgEYecP8LVA1ioO4FkgRQ@mail.gmail.com>
On 11/25/14 21:36, Scott Feldman wrote:
> On Tue, Nov 25, 2014 at 6:50 AM, Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>> On 11/25/14 11:30, John Fastabend wrote:
>> Ok, guess i am gonna have to go stare at the code some more.
>> I thought we returned one of the error codes?
>> A bitmask would work for a single entry - because you have two
>> options add to h/ware and/or s/ware. So response is easy to encode.
>> But if i have 1000 and they are sparsely populated (think an indexed
>> table and i have indices 1, 23, 45, etc), then a bitmask would be
>> hard to use.
>
> I'm confused by this discussion.
This is about the policy which states "install as many as you can, dont
worry about failures". In such a case, how do you tell user space back
"oh, btw you know your request #1, #23, and 45 went ok, but nothing else
worked". A simple return code wont work. You could return a code to
say "some worked". At which case user space could dump and find out only
#1, #23 and #45 worked.
Your question below is a different context; Some people may want
the policy where whats in hardware
a) gets to be seen in software and b) allow for destination lookup
failures in hardware to show up in the kernel, refresh the fdb in the
kernel via learning
and c) whats in s.ware gets synced to hardware just because there's
space in the hardware
I dont want any of the above;-> Which would work if we had policy knobs.
Learning, flooding, sync from hardware. Speaking of the last one:
Where is my cookie Scott? I want my cookie.
cheers,
jamal
> Do I have this right: You want to
> send 1000 RTM_NEWNEIGHs to PF_BRIDGE with both NTF_MASTER and NTF_SELF
> set such that 1000 new FBD entries are installed in both (SW) the
> bridge's FDB and (HW) the port driver's FDB. My first confusion is
> why do you want these FBD entries in bridge's FDB? We're offloading
> the switching to HW so HW should be handling fwd plane. If ctrl pkt
> make it to SW, it can learn those FDB entries;
> no need for manual
> install of FDB entry in SW. It seems to me you only want to use
> NTF_SELF to install the FDB entry in HW using the port driver. And an
> error code is returned for that install. Since there is only one
> target (NTF_SELF) there is no need for bitmask return.
^ permalink raw reply
* Re: [net-next PATCH 2/5] ethernet/intel: Use eth_skb_pad helper
From: David Miller @ 2014-11-26 3:19 UTC (permalink / raw)
To: eric.dumazet
Cc: alexander.duyck, alexander.h.duyck, netdev, jeffrey.t.kirsher
In-Reply-To: <1416966185.29427.46.camel@edumazet-glaptop2.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 25 Nov 2014 17:43:05 -0800
> I believe I finally have an idea why we had various + 15 in skb
> allocations in TCP stack !
It was so that you could do one level of tunneling with "for
free". Or that is my recollection.
Those + 15 existed way before any of these padto() calls even
existed.
^ permalink raw reply
* Re: [PATCH 3/4] GMAC: dts: add gmac info for rk3288
From: Roger @ 2014-11-26 2:49 UTC (permalink / raw)
To: Heiko Stübner
Cc: Sergei Shtylyov, peppe.cavallaro, netdev, linux-kernel,
linux-rockchip, kever.yang, mark.yao, eddie.cai
In-Reply-To: <1773507.kOcjDCLbcs@diego>
On 2014/11/25 22:39, Heiko Stübner wrote:
> Am Dienstag, 25. November 2014, 16:40:59 schrieb Sergei Shtylyov:
>> Hello.
>>
>> On 11/25/2014 12:08 PM, Roger Chen wrote:
>>> add gmac info in rk3288.dtsi for GMAC driver
>>>
>>> Signed-off-by: Roger Chen <roger.chen@rock-chips.com>
>>> ---
>>>
>>> arch/arm/boot/dts/rk3288.dtsi | 59
>>> +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 59
>>> insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
>>> index 0f50d5d..949675d 100644
>>> --- a/arch/arm/boot/dts/rk3288.dtsi
>>> +++ b/arch/arm/boot/dts/rk3288.dtsi
>> [...]
>>
>>> @@ -490,6 +497,25 @@
>>>
>>> reg = <0xff740000 0x1000>;
>>>
>>> };
>>>
>>> + gmac: eth@ff290000 {
>> Please name the node "ethernet@ff290000" to comply with the ePAPR
>> standard.
>>> + compatible = "rockchip,rk3288-gmac";
>>> + reg = <0xff290000 0x10000>;
>>> + interrupts = <GIC_SPI 27 IRQ_TYPE_LEVEL_HIGH>; /*irq=59*/
>>> + interrupt-names = "macirq";
>>> + rockchip,grf = <&grf>;
>>> + clocks = <&cru SCLK_MAC>, <&cru SCLK_MAC_PLL>,
>>> + <&cru SCLK_MAC_RX>, <&cru SCLK_MAC_TX>,
>>> + <&cru SCLK_MACREF>, <&cru SCLK_MACREF_OUT>,
>>> + <&cru ACLK_GMAC>, <&cru PCLK_GMAC>;
>>> + clock-names = "stmmaceth", "clk_mac_pll",
>>> + "mac_clk_rx", "mac_clk_tx",
>>> + "clk_mac_ref", "clk_mac_refout",
>>> + "aclk_mac", "pclk_mac";
>>> + phy-mode = "rgmii";
>>> + pinctrl-names = "default";
>>> + pinctrl-0 = <&rgmii_pin /*&rmii_pin*/>;
>> Hm, pinctrl props in a .dtsi file? Those are usually board dependent.
> yep, especially as there is a board-dependent selection needed of what to use
> [rgmii or rmii] depending on the phy on the board.
of course pinctrl props is redefined in the rk3288-evb-rk808.dts,
IMO, pinctrl props in .dtsi is used to fit for the "phy-mode" prop as default.
if pinctrl props should be removed, how about "phy-mode" prop?
>> [...]
>>
>>> @@ -1040,5 +1066,38 @@
>>>
>>> rockchip,pins = <7 23 3 &pcfg_pull_none>;
>>>
>>> };
>>>
>>> };
>>>
>>> +
>>> + gmac {
>>> + rgmii_pin: rgmii-pins {
> please add the "s" to the label - "rgmii_pins"
>
>
>>> + rockchip,pins = <3 30 3 &pcfg_pull_none>,
>>> + <3 31 3 &pcfg_pull_none>,
>>> + <3 26 3 &pcfg_pull_none>,
>>> + <3 27 3 &pcfg_pull_none>,
>>> + <3 28 3 &pcfg_pull_none>,
>>> + <3 29 3 &pcfg_pull_none>,
>>> + <3 24 3 &pcfg_pull_none>,
>>> + <3 25 3 &pcfg_pull_none>,
>>> + <4 0 3 &pcfg_pull_none>,
>>> + <4 5 3 &pcfg_pull_none>,
>>> + <4 6 3 &pcfg_pull_none>,
>>> + <4 9 3 &pcfg_pull_none>,
>>> + <4 4 3 &pcfg_pull_none>,
>>> + <4 1 3 &pcfg_pull_none>,
>>> + <4 3 3 &pcfg_pull_none>;
>>> + };
>>> +
>>> + rmii_pin: rmii-pins {
> same here
>
>
>>> + rockchip,pins = <3 30 3 &pcfg_pull_none>,
>>> + <3 31 3 &pcfg_pull_none>,
>>> + <3 28 3 &pcfg_pull_none>,
>>> + <3 29 3 &pcfg_pull_none>,
>>> + <4 0 3 &pcfg_pull_none>,
>>> + <4 5 3 &pcfg_pull_none>,
>>> + <4 4 3 &pcfg_pull_none>,
>>> + <4 1 3 &pcfg_pull_none>,
>>> + <4 2 3 &pcfg_pull_none>,
>>> + <4 3 3 &pcfg_pull_none>;
>>> + };
>>> + };
>> These are usually define in the board .dts file...
> The pinctrl settings itself are soc-specific, i.e. the pins and their settings
> to use to enable r{g}mii functionality are the same for all boards using this
> soc, so the pinctrl definitions should stay here and not be redefined in each
> and every board file.
>
>
> Heiko
>
>
>
^ permalink raw reply
* Re: [net-next PATCH 1/5] etherdevice: Add function for handling padding frame to ETH_ZLEN
From: Alexander Duyck @ 2014-11-26 2:44 UTC (permalink / raw)
To: Florian Fainelli, Alexander Duyck, netdev; +Cc: davem
In-Reply-To: <54752525.4040509@gmail.com>
On 11/25/2014 04:56 PM, Florian Fainelli wrote:
> On 25/11/14 14:44, Alexander Duyck wrote:
>> This patch adds a simple function for padding a frame up to the minimum
>> size for for Ethernet. The motivation behind it is that there are a number
>> of implementations throughout the network device drivers that are all doing
>> the same thing, but each a little bit differently and as a result several
>> implementations contain bugs such as updating the length without updating
>> the tail offset and other similar issues.
>>
>> Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
>> ---
>> include/linux/etherdevice.h | 21 +++++++++++++++++++++
>> 1 file changed, 21 insertions(+)
>>
>> diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h
>> index 733980f..7e436f3 100644
>> --- a/include/linux/etherdevice.h
>> +++ b/include/linux/etherdevice.h
>> @@ -392,4 +392,25 @@ static inline unsigned long compare_ether_header(const void *a, const void *b)
>> #endif
>> }
>>
>> +/**
>> + * eth_skb_pad - Pad buffer to mininum number of octets for Ethernet frame
>> + * @skb: Buffer to pad
>> + *
>> + * An Ethernet frame should have a minimum size of 60 bytes. This function
>> + * takes short frames and pads them with zeros up to the 60 byte limit.
>
> minimum size without FCS
>
>> + */
>> +static inline int eth_skb_pad(struct sk_buff *skb)
>> +{
>> + unsigned int size = skb->len;
>> +
>> + if (unlikely(size < ETH_ZLEN)) {
>> + size = ETH_ZLEN - size;
>> + if (skb_pad(skb, size))
>> + return -ENOMEM;
>> + __skb_put(skb, size);
>> + }
>
> most drivers call skb_padto(skb, ETH_ZLEN), besides the extra
> __skb_put() here, this is not different.
Yes, but the __skb_put is the difference. Calling skb_padto() only
zeros out the bytes following the tail, but it doesn't add the extra
bytes to the frame nor does it update the tail. As such if we only call
skb_padto() before handing a short frame off to the network stack the
network stack will still have issues with it being too short.
The difference between the two is that skb_padto() is really meant for
transmit, whereas eth_skb_pad is really meant for receive.
- Alex
^ permalink raw reply
* Re: [patch net-next v3 09/17] bridge: add API to notify bridge driver of learned FBD on offloaded device
From: Scott Feldman @ 2014-11-26 2:40 UTC (permalink / raw)
To: Florian Fainelli
Cc: Jiri Pirko, Netdev, David S. Miller, nhorman@tuxdriver.com,
Andy Gospodarek, Thomas Graf, dborkman@redhat.com,
ogerlitz@mellanox.com, jesse@nicira.com, pshelar@nicira.com,
azhou@nicira.com, ben@decadent.org.uk, stephen@networkplumber.org,
Kirsher, Jeffrey T, vyasevic@redhat.com, Cong Wang,
Fastabend, John R, Eric Dumazet, Jamal Hadi Salim, Roopa Prabhu,
John Linville
In-Reply-To: <54753C1F.1080109@gmail.com>
On Tue, Nov 25, 2014 at 4:34 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> On 25/11/14 18:03, Scott Feldman wrote:
>> On Tue, Nov 25, 2014 at 12:44 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
>>> On 25/11/14 02:28, Jiri Pirko wrote:
>>>> From: Scott Feldman <sfeldma@gmail.com>
>>>>
>>>> When the swdev device learns a new mac/vlan on a port, it sends some async
>>>> notification to the driver and the driver installs an FDB in the device.
>>>> To give a holistic system view, the learned mac/vlan should be reflected
>>>> in the bridge's FBD table, so the user, using normal iproute2 cmds, can view
>>>> what is currently learned by the device. This API on the bridge driver gives
>>>> a way for the swdev driver to install an FBD entry in the bridge FBD table.
>>>> (And remove one).
>>>>
>>>> This is equivalent to the device running these cmds:
>>>>
>>>> bridge fdb [add|del] <mac> dev <dev> vid <vlan id> master
>>>>
>>>> This patch needs some extra eyeballs for review, in paricular around the
>>>> locking and contexts.
>>>>
>>>> Signed-off-by: Scott Feldman <sfeldma@gmail.com>
>>>> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>>>> ---
>>>
>>> [snip]
>>>
>>>> + head = &br->hash[br_mac_hash(addr, vid)];
>>>> + fdb = fdb_find(head, addr, vid);
>>>> + if (!fdb) {
>>>> + fdb = fdb_create(head, p, addr, vid);
>>>> + if (!fdb) {
>>>> + err = -ENOMEM;
>>>> + goto err_unlock;
>>>> + }
>>>> + fdb->added_by_external_learn = 1;
>>>> + fdb_notify(br, fdb, RTM_NEWNEIGH);
>>>> + } else if (fdb->added_by_external_learn) {
>>>> + /* Refresh entry */
>>>> + fdb->updated = fdb->used = jiffies;
>>>> + } else if (!fdb->added_by_user) {
>>>> + /* Take over SW learned entry */
>>>> + fdb->added_by_external_learn = 1;
>>>> + fdb->updated = jiffies;
>>>> + fdb_notify(br, fdb, RTM_NEWNEIGH);
>>>> + }
>>>
>>> Is there any case where this fdb entry gets re-used and is no longer
>>> added by an external learning? Should we clear this flag somewhere?
>>
>> Once the FDB entry is marked "added_by_external_learn" it stays marked
>> as such until removed by aging cleanup process (or flushed due to
>> interface down, etc). If aged out (and now deleted), the FDB entry
>> may come back either by SW learn or by HW learn. If SW learn comes
>> first, and then HW learn, HW learn will override and mark the existing
>> FDB entry "added_by_external_learn". So there is take-over by HW but
>> no give-back to SW. And there is no explicit clearing of the mark
>> short of deleting the FDB entry. The mark is mostly for letting
>> user's know which FDB entries where learned by HW and synced to the
>> bridge's FDB.
>
> Thanks, makes sense now. This is probably obvious in this context, but
> maybe it would not hurt to come up with a documentation that describe
> the offload API, FDB entry lifetime and HW/SW ownership etc...
I have an updated Documentation/networking/switchdev.txt that covers
the swdev APIs and usage and notes, but Jiri is being stingy with it.
Will get this out, either in v4 or follow-on patches. There is enough
going on just with L2 offload that we're going to need some good
documentation to guide implementers.
^ permalink raw reply
* [PATCH] stmmac: platform: fix default values of the filter bins setting
From: Huacai Chen @ 2014-11-26 2:38 UTC (permalink / raw)
To: Giuseppe Cavallaro; +Cc: Vince Bridgers, David S. Miller, netdev, Huacai Chen
The commit 3b57de958e2a brought the support for a different amount of
the filter bins, but didn't update the platform driver that without
CONFIG_OF.
Fixes: 3b57de958e2a (net: stmmac: Support devicetree configs for mcast
and ucast filter entries)
Signed-off-by: Huacai Chen <chenhc@lemote.com>
---
.../net/ethernet/stmicro/stmmac/stmmac_platform.c | 13 +++++++------
1 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index db56fa7..5b0da39 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -177,12 +177,6 @@ static int stmmac_probe_config_dt(struct platform_device *pdev,
*/
plat->maxmtu = JUMBO_LEN;
- /* Set default value for multicast hash bins */
- plat->multicast_filter_bins = HASH_TABLE_SIZE;
-
- /* Set default value for unicast filter entries */
- plat->unicast_filter_entries = 1;
-
/*
* Currently only the properties needed on SPEAr600
* are provided. All other properties should be added
@@ -270,6 +264,13 @@ static int stmmac_pltfr_probe(struct platform_device *pdev)
return PTR_ERR(addr);
plat_dat = dev_get_platdata(&pdev->dev);
+
+ /* Set default value for multicast hash bins */
+ plat_dat->multicast_filter_bins = HASH_TABLE_SIZE;
+
+ /* Set default value for unicast filter entries */
+ plat_dat->unicast_filter_entries = 1;
+
if (pdev->dev.of_node) {
if (!plat_dat)
plat_dat = devm_kzalloc(&pdev->dev,
--
1.7.7.3
^ permalink raw reply related
* Re: [patch net-next v3 02/17] net: make vid as a parameter for ndo_fdb_add/ndo_fdb_del
From: Scott Feldman @ 2014-11-26 2:36 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: John Fastabend, Jiri Pirko, Netdev, David S. Miller,
nhorman@tuxdriver.com, Andy Gospodarek, Thomas Graf,
dborkman@redhat.com, ogerlitz@mellanox.com, jesse@nicira.com,
pshelar@nicira.com, azhou@nicira.com, ben@decadent.org.uk,
stephen@networkplumber.org, Kirsher, Jeffrey T,
vyasevic@redhat.com, Cong Wang, Eric Dumazet, Florian Fainelli,
Roopa Prabhu, John Linville
In-Reply-To: <5474B353.10802@mojatatu.com>
On Tue, Nov 25, 2014 at 6:50 AM, Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> On 11/25/14 11:30, John Fastabend wrote:
>>
>> On 11/25/2014 08:18 AM, Jamal Hadi Salim wrote:
>>>
>>> On 11/25/14 11:01, John Fastabend wrote:
>>>>
>>>> On 11/25/2014 07:38 AM, Jamal Hadi Salim wrote:
>>>>>
>>>>> On 11/25/14 05:28, Jiri Pirko wrote:
>>>>>>
>>>>>> Do the work of parsing NDA_VLAN directly in rtnetlink code, pass
>>>>>> simple
>>>>>> u16 vid to drivers from there.
>>>>>>
>>>>>
>
>
>> Actually (after having some coffee) this becomes much more useful
>> if you return which items failed. Then you can slam the hardware
>> with your 100 entries, probably a lot more then that, and come back
>> later and clean it up.
>>
>
> Yes, that is the general use case.
> Unfortunately at the moment we only return codes on a netlink set
> direction - but would be a beauty if we could return what succeeded
> and didnt in some form of vector.
> Note: all is not lost because you can always do a get afterwards and
> find what is missing if you got a return code of "partial success".
> Just a little less efficient..
>
>
>> We return a bitmask of which operations were successful. So if SW fails
>> we have both bits cleared and we abort. When SW is successful we set the
>> SW bit and try to program the HW. If its sucessful we set the HW bit if
>> its not we abort with an err. Converting this to (1) is not much work
>> just skip the abort.
>>
>
> Ok, guess i am gonna have to go stare at the code some more.
> I thought we returned one of the error codes?
> A bitmask would work for a single entry - because you have two
> options add to h/ware and/or s/ware. So response is easy to encode.
> But if i have 1000 and they are sparsely populated (think an indexed
> table and i have indices 1, 23, 45, etc), then a bitmask would be
> hard to use.
I'm confused by this discussion. Do I have this right: You want to
send 1000 RTM_NEWNEIGHs to PF_BRIDGE with both NTF_MASTER and NTF_SELF
set such that 1000 new FBD entries are installed in both (SW) the
bridge's FDB and (HW) the port driver's FDB. My first confusion is
why do you want these FBD entries in bridge's FDB? We're offloading
the switching to HW so HW should be handling fwd plane. If ctrl pkt
make it to SW, it can learn those FDB entries; no need for manual
install of FDB entry in SW. It seems to me you only want to use
NTF_SELF to install the FDB entry in HW using the port driver. And an
error code is returned for that install. Since there is only one
target (NTF_SELF) there is no need for bitmask return.
> cheers,
> jamal
^ permalink raw reply
* Re: [patch net-next v3 09/17] bridge: add API to notify bridge driver of learned FBD on offloaded device
From: Florian Fainelli @ 2014-11-26 2:34 UTC (permalink / raw)
To: Scott Feldman
Cc: Jiri Pirko, Netdev, David S. Miller, nhorman@tuxdriver.com,
Andy Gospodarek, Thomas Graf, dborkman@redhat.com,
ogerlitz@mellanox.com, jesse@nicira.com, pshelar@nicira.com,
azhou@nicira.com, ben@decadent.org.uk, stephen@networkplumber.org,
Kirsher, Jeffrey T, vyasevic@redhat.com, Cong Wang,
Fastabend, John R, Eric Dumazet, Jamal Hadi Salim, Roopa Prabhu,
John Linville
In-Reply-To: <CAE4R7bBQnxKh7y+ViB-s-7b7Ro=gTQLavUTcNLA1u=3UJHa1Rg@mail.gmail.com>
On 25/11/14 18:03, Scott Feldman wrote:
> On Tue, Nov 25, 2014 at 12:44 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
>> On 25/11/14 02:28, Jiri Pirko wrote:
>>> From: Scott Feldman <sfeldma@gmail.com>
>>>
>>> When the swdev device learns a new mac/vlan on a port, it sends some async
>>> notification to the driver and the driver installs an FDB in the device.
>>> To give a holistic system view, the learned mac/vlan should be reflected
>>> in the bridge's FBD table, so the user, using normal iproute2 cmds, can view
>>> what is currently learned by the device. This API on the bridge driver gives
>>> a way for the swdev driver to install an FBD entry in the bridge FBD table.
>>> (And remove one).
>>>
>>> This is equivalent to the device running these cmds:
>>>
>>> bridge fdb [add|del] <mac> dev <dev> vid <vlan id> master
>>>
>>> This patch needs some extra eyeballs for review, in paricular around the
>>> locking and contexts.
>>>
>>> Signed-off-by: Scott Feldman <sfeldma@gmail.com>
>>> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>>> ---
>>
>> [snip]
>>
>>> + head = &br->hash[br_mac_hash(addr, vid)];
>>> + fdb = fdb_find(head, addr, vid);
>>> + if (!fdb) {
>>> + fdb = fdb_create(head, p, addr, vid);
>>> + if (!fdb) {
>>> + err = -ENOMEM;
>>> + goto err_unlock;
>>> + }
>>> + fdb->added_by_external_learn = 1;
>>> + fdb_notify(br, fdb, RTM_NEWNEIGH);
>>> + } else if (fdb->added_by_external_learn) {
>>> + /* Refresh entry */
>>> + fdb->updated = fdb->used = jiffies;
>>> + } else if (!fdb->added_by_user) {
>>> + /* Take over SW learned entry */
>>> + fdb->added_by_external_learn = 1;
>>> + fdb->updated = jiffies;
>>> + fdb_notify(br, fdb, RTM_NEWNEIGH);
>>> + }
>>
>> Is there any case where this fdb entry gets re-used and is no longer
>> added by an external learning? Should we clear this flag somewhere?
>
> Once the FDB entry is marked "added_by_external_learn" it stays marked
> as such until removed by aging cleanup process (or flushed due to
> interface down, etc). If aged out (and now deleted), the FDB entry
> may come back either by SW learn or by HW learn. If SW learn comes
> first, and then HW learn, HW learn will override and mark the existing
> FDB entry "added_by_external_learn". So there is take-over by HW but
> no give-back to SW. And there is no explicit clearing of the mark
> short of deleting the FDB entry. The mark is mostly for letting
> user's know which FDB entries where learned by HW and synced to the
> bridge's FDB.
Thanks, makes sense now. This is probably obvious in this context, but
maybe it would not hurt to come up with a documentation that describe
the offload API, FDB entry lifetime and HW/SW ownership etc...
>
>> [snip]
>>
>>> +EXPORT_SYMBOL(br_fdb_external_learn_del);
>>> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
>>> index 4f577c4..02cd63b 100644
>>> --- a/net/bridge/br_private.h
>>> +++ b/net/bridge/br_private.h
>>> @@ -101,6 +101,7 @@ struct net_bridge_fdb_entry
>>> unsigned char is_local;
>>> unsigned char is_static;
>>> unsigned char added_by_user;
>>> + unsigned char added_by_external_learn;
>>
>> Pheww, we could be saving some memory footprint here by using different
>> types here ;)
>> --
>> Florian
^ permalink raw reply
* Re: [PATCH 1/4] GMAC: add driver for Rockchip RK3288 SoCs integrated GMAC
From: Roger @ 2014-11-26 2:29 UTC (permalink / raw)
To: Giuseppe CAVALLARO, heiko
Cc: netdev, linux-kernel, linux-rockchip, kever.yang, mark.yao,
eddie.cai
In-Reply-To: <54745455.9060607@st.com>
Hi! Giuseppe CAVALLARO
在 2014/11/25 18:05, Giuseppe CAVALLARO 写道:
> Hello Roger
>
> thx for these patches, my comments inline below
>
> On 11/25/2014 10:07 AM, Roger Chen wrote:
>> This driver is based on stmmac driver.
>>
>> Signed-off-by: Roger Chen <roger.chen@rock-chips.com>
>> ---
>> drivers/net/ethernet/stmicro/stmmac/Makefile | 2 +-
>> drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 629
>> ++++++++++++++++++++
>> .../net/ethernet/stmicro/stmmac/stmmac_platform.c | 3 +
>> .../net/ethernet/stmicro/stmmac/stmmac_platform.h | 1 +
>> 4 files changed, 634 insertions(+), 1 deletion(-)
>> create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile
>> b/drivers/net/ethernet/stmicro/stmmac/Makefile
>> index ac4d562..73c2715 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/Makefile
>> +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
>> @@ -6,7 +6,7 @@ stmmac-objs:= stmmac_main.o stmmac_ethtool.o
>> stmmac_mdio.o ring_mode.o \
>>
>> obj-$(CONFIG_STMMAC_PLATFORM) += stmmac-platform.o
>> stmmac-platform-objs:= stmmac_platform.o dwmac-meson.o
>> dwmac-sunxi.o \
>> - dwmac-sti.o dwmac-socfpga.o
>> + dwmac-sti.o dwmac-socfpga.o dwmac-rk.o
>>
>> obj-$(CONFIG_STMMAC_PCI) += stmmac-pci.o
>> stmmac-pci-objs:= stmmac_pci.o
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>> b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>> new file mode 100644
>> index 0000000..9e50b37
>> --- /dev/null
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>> @@ -0,0 +1,629 @@
>> +/**
>> + * dwmac-rk.c - Rockchip RK3288 DWMAC specific glue layer
>> + *
>> + * Copyright (C) 2014 Chen-Zhi (Roger Chen)
>> + *
>> + * Chen-Zhi (Roger Chen) <roger.chen@rock-chips.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#include <linux/stmmac.h>
>> +#include <linux/clk.h>
>> +#include <linux/phy.h>
>> +#include <linux/of_net.h>
>> +#include <linux/gpio.h>
>> +#include <linux/of_gpio.h>
>> +#include <linux/of_device.h>
>> +#include <linux/regulator/consumer.h>
>> +#include <linux/delay.h>
>> +#include <linux/regmap.h>
>> +#include <linux/mfd/syscon.h>
>> +
>> +struct rk_priv_data {
>> + struct platform_device *pdev;
>> + int phy_iface;
>> + bool power_ctrl_by_pmu;
>> + char pmu_regulator[32];
>> + int pmu_enable_level;
>> +
>> + int power_io;
>> + int power_io_level;
>> + int reset_io;
>> + int reset_io_level;
>> + int phyirq_io;
>> + int phyirq_io_level;
>> +
>> + bool clk_enabled;
>> + bool clock_input;
>> +
>> + struct clk *clk_mac;
>> + struct clk *clk_mac_pll;
>> + struct clk *gmac_clkin;
>> + struct clk *mac_clk_rx;
>> + struct clk *mac_clk_tx;
>> + struct clk *clk_mac_ref;
>> + struct clk *clk_mac_refout;
>> + struct clk *aclk_mac;
>> + struct clk *pclk_mac;
>> +
>> + int tx_delay;
>> + int rx_delay;
>> +
>> + struct regmap *grf;
>> +};
>> +
>> +#define RK3288_GRF_SOC_CON1 0x0248
>> +#define RK3288_GRF_SOC_CON3 0x0250
>> +#define RK3288_GRF_GPIO3D_E 0x01ec
>> +#define RK3288_GRF_GPIO4A_E 0x01f0
>> +#define RK3288_GRF_GPIO4B_E 0x01f4
>> +
>> +/*RK3288_GRF_SOC_CON1*/
>> +#define GMAC_PHY_INTF_SEL_RGMII ((0x01C0 << 16) | (0x0040))
>> +#define GMAC_PHY_INTF_SEL_RMII ((0x01C0 << 16) | (0x0100))
>> +#define GMAC_FLOW_CTRL ((0x0200 << 16) | (0x0200))
>> +#define GMAC_FLOW_CTRL_CLR ((0x0200 << 16) | (0x0000))
>> +#define GMAC_SPEED_10M ((0x0400 << 16) | (0x0000))
>> +#define GMAC_SPEED_100M ((0x0400 << 16) | (0x0400))
>> +#define GMAC_RMII_CLK_25M ((0x0800 << 16) | (0x0800))
>> +#define GMAC_RMII_CLK_2_5M ((0x0800 << 16) | (0x0000))
>> +#define GMAC_CLK_125M ((0x3000 << 16) | (0x0000))
>> +#define GMAC_CLK_25M ((0x3000 << 16) | (0x3000))
>> +#define GMAC_CLK_2_5M ((0x3000 << 16) | (0x2000))
>> +#define GMAC_RMII_MODE ((0x4000 << 16) | (0x4000))
>> +#define GMAC_RMII_MODE_CLR ((0x4000 << 16) | (0x0000))
>> +
>> +/*RK3288_GRF_SOC_CON3*/
>> +#define GMAC_TXCLK_DLY_ENABLE ((0x4000 << 16) | (0x4000))
>> +#define GMAC_TXCLK_DLY_DISABLE ((0x4000 << 16) | (0x0000))
>> +#define GMAC_RXCLK_DLY_ENABLE ((0x8000 << 16) | (0x8000))
>> +#define GMAC_RXCLK_DLY_DISABLE ((0x8000 << 16) | (0x0000))
>> +#define GMAC_CLK_RX_DL_CFG(val) ((0x3F80 << 16) | (val<<7))
>> +#define GMAC_CLK_TX_DL_CFG(val) ((0x007F << 16) | (val))
>
> why do not use BIT and BIT_MASK where possible?
>
>> +static void set_to_rgmii(struct rk_priv_data *bsp_priv,
>> + int tx_delay, int rx_delay)
>> +{
>> + if (IS_ERR(bsp_priv->grf)) {
>> + pr_err("%s: Missing rockchip,grf property\n", __func__);
>> + return;
>> + }
>> +
>> + regmap_write(bsp_priv->grf, RK3288_GRF_SOC_CON1,
>> + GMAC_PHY_INTF_SEL_RGMII);
>> + regmap_write(bsp_priv->grf, RK3288_GRF_SOC_CON1,
>> + GMAC_RMII_MODE_CLR);
>
> maybe you could perform just one write unless there is some HW
> constraint.
>
>> + regmap_write(bsp_priv->grf, RK3288_GRF_SOC_CON3,
>> + GMAC_RXCLK_DLY_ENABLE);
>> + regmap_write(bsp_priv->grf, RK3288_GRF_SOC_CON3,
>> + GMAC_TXCLK_DLY_ENABLE);
>> + regmap_write(bsp_priv->grf, RK3288_GRF_SOC_CON3,
>> + GMAC_CLK_RX_DL_CFG(rx_delay));
>> + regmap_write(bsp_priv->grf, RK3288_GRF_SOC_CON3,
>> + GMAC_CLK_TX_DL_CFG(tx_delay));
>
> ditto
>
>> +
>> + regmap_write(bsp_priv->grf, RK3288_GRF_GPIO3D_E, 0xFFFFFFFF);
>> + regmap_write(bsp_priv->grf, RK3288_GRF_GPIO4B_E,
>> + 0x3<<2<<16 | 0x3<<2);
>
> pls use macros, these shift sequence is really help to decode
>
>> + regmap_write(bsp_priv->grf, RK3288_GRF_GPIO4A_E, 0xFFFFFFFF);
>> +
>> + pr_info("%s: tx delay=0x%x; rx delay=0x%x;\n",
>> + __func__, tx_delay, rx_delay);
>
> may I suggest pr_debug?
>
>> +}
>> +
>> +static void set_to_rmii(struct rk_priv_data *bsp_priv)
>> +{
>> + if (IS_ERR(bsp_priv->grf)) {
>> + pr_err("%s: Missing rockchip,grf property\n", __func__);
>> + return;
>> + }
>> +
>> + regmap_write(bsp_priv->grf, RK3288_GRF_SOC_CON1,
>> + GMAC_PHY_INTF_SEL_RMII);
>> + regmap_write(bsp_priv->grf, RK3288_GRF_SOC_CON1,
>> + GMAC_RMII_MODE);
>> +}
>> +
>> +static void set_rgmii_speed(struct rk_priv_data *bsp_priv, int speed)
>> +{
>> + if (IS_ERR(bsp_priv->grf)) {
>> + pr_err("%s: Missing rockchip,grf property\n", __func__);
>> + return;
>> + }
>> +
>> + if (speed == 10)
>> + regmap_write(bsp_priv->grf, RK3288_GRF_SOC_CON1,
>> GMAC_CLK_2_5M);
>> + else if (speed == 100)
>> + regmap_write(bsp_priv->grf, RK3288_GRF_SOC_CON1, GMAC_CLK_25M);
>> + else if (speed == 1000)
>> + regmap_write(bsp_priv->grf, RK3288_GRF_SOC_CON1,
>> GMAC_CLK_125M);
>> + else
>> + pr_err("unknown speed value for RGMII! speed=%d", speed);
>> +}
>> +
>> +static void set_rmii_speed(struct rk_priv_data *bsp_priv, int speed)
>> +{
>> + if (IS_ERR(bsp_priv->grf)) {
>> + pr_err("%s: Missing rockchip,grf property\n", __func__);
>> + return;
>> + }
>> +
>> + if (speed == 10) {
>> + regmap_write(bsp_priv->grf, RK3288_GRF_SOC_CON1,
>> + GMAC_RMII_CLK_2_5M);
>> + regmap_write(bsp_priv->grf, RK3288_GRF_SOC_CON1,
>> + GMAC_SPEED_10M);
>> + } else if (speed == 100) {
>> + regmap_write(bsp_priv->grf, RK3288_GRF_SOC_CON1,
>> + GMAC_RMII_CLK_25M);
>> + regmap_write(bsp_priv->grf, RK3288_GRF_SOC_CON1,
>> + GMAC_SPEED_100M);
>> + } else {
>> + pr_err("unknown speed value for RMII! speed=%d", speed);
>> + }
>> +}
>> +
>> +#define MAC_CLK_RX "mac_clk_rx"
>> +#define MAC_CLK_TX "mac_clk_tx"
>> +#define CLK_MAC_REF "clk_mac_ref"
>> +#define CLK_MAC_REF_OUT "clk_mac_refout"
>> +#define CLK_MAC_PLL "clk_mac_pll"
>> +#define ACLK_MAC "aclk_mac"
>> +#define PCLK_MAC "pclk_mac"
>> +#define MAC_CLKIN "ext_gmac"
>> +#define CLK_MAC "stmmaceth"
>
> Concerning the clocks, the "stmmaceth" is actually used for the main
> clock so maybe you should use another name.
>
in stmmac_main.c, clk "stmmaceth"(STMMAC_RESOURCE_NAME) is used to be
the source of priv->clk_csr.
priv->stmmac_clk = devm_clk_get(priv->device, STMMAC_RESOURCE_NAME);
so I have to define a kind of clock named "stmmaceth".
> See: Documentation/devicetree/bindings/net/stmmac.txt
>
> For St platforms, it has been used: sti-ethclk but we could
> used a common and better name: stmmac-phyclk
> And I could also propose this change on STi glue-logic.
>
> Then, I think that all the clock macros above should be documented
> and maybe managed at DT level, what do you think?
>
>> +
>> +int gmac_clk_init(struct rk_priv_data *bsp_priv)
>> +{
>> + struct device *dev = &bsp_priv->pdev->dev;
>> +
>> + bsp_priv->clk_enabled = false;
>> +
>> + bsp_priv->mac_clk_rx = clk_get(dev, MAC_CLK_RX);
>> + if (IS_ERR(bsp_priv->mac_clk_rx))
>> + pr_warn("%s: warning: cannot get clock %s\n",
>> + __func__, MAC_CLK_RX);
>> +
>> + bsp_priv->mac_clk_tx = clk_get(dev, MAC_CLK_TX);
>> + if (IS_ERR(bsp_priv->mac_clk_tx))
>> + pr_warn("%s: warning: cannot get clock %s\n",
>> + __func__, MAC_CLK_TX);
>> +
>> + bsp_priv->clk_mac_ref = clk_get(dev, CLK_MAC_REF);
>> + if (IS_ERR(bsp_priv->clk_mac_ref))
>> + pr_warn("%s: warning: cannot get clock %s\n",
>> + __func__, CLK_MAC_REF);
>> +
>> + bsp_priv->clk_mac_refout = clk_get(dev, CLK_MAC_REF_OUT);
>> + if (IS_ERR(bsp_priv->clk_mac_refout))
>> + pr_warn("%s: warning:cannot get clock %s\n",
>> + __func__, CLK_MAC_REF_OUT);
>> +
>> + bsp_priv->aclk_mac = clk_get(dev, ACLK_MAC);
>> + if (IS_ERR(bsp_priv->aclk_mac))
>> + pr_warn("%s: warning: cannot get clock %s\n",
>> + __func__, ACLK_MAC);
>> +
>> + bsp_priv->pclk_mac = clk_get(dev, PCLK_MAC);
>> + if (IS_ERR(bsp_priv->pclk_mac))
>> + pr_warn("%s: warning: cannot get clock %s\n",
>> + __func__, PCLK_MAC);
>> +
>> + bsp_priv->clk_mac_pll = clk_get(dev, CLK_MAC_PLL);
>> + if (IS_ERR(bsp_priv->clk_mac_pll))
>> + pr_warn("%s: warning: cannot get clock %s\n",
>> + __func__, CLK_MAC_PLL);
>> +
>> + bsp_priv->gmac_clkin = clk_get(dev, MAC_CLKIN);
>> + if (IS_ERR(bsp_priv->gmac_clkin))
>> + pr_warn("%s: warning: cannot get clock %s\n",
>> + __func__, MAC_CLKIN);
>> +
>> + bsp_priv->clk_mac = clk_get(dev, CLK_MAC);
>> + if (IS_ERR(bsp_priv->clk_mac))
>> + pr_warn("%s: warning: cannot get clock %s\n",
>> + __func__, CLK_MAC);
>> +
>> + if (bsp_priv->clock_input) {
>> + pr_info("%s: clock input from PHY\n", __func__);
>> + } else {
>> + if (bsp_priv->phy_iface == PHY_INTERFACE_MODE_RMII)
>> + clk_set_rate(bsp_priv->clk_mac_pll, 50000000);
>> +
>> + clk_set_parent(bsp_priv->clk_mac, bsp_priv->clk_mac_pll);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int gmac_clk_enable(struct rk_priv_data *bsp_priv, bool enable)
>> +{
>> + int phy_iface = phy_iface = bsp_priv->phy_iface;
>> +
>> + if (enable) {
>> + if (!bsp_priv->clk_enabled) {
>> + if (phy_iface == PHY_INTERFACE_MODE_RMII) {
>> + if (!IS_ERR(bsp_priv->mac_clk_rx))
>> + clk_prepare_enable(
>> + bsp_priv->mac_clk_rx);
>> +
>> + if (!IS_ERR(bsp_priv->clk_mac_ref))
>> + clk_prepare_enable(
>> + bsp_priv->clk_mac_ref);
>> +
>> + if (!IS_ERR(bsp_priv->clk_mac_refout))
>> + clk_prepare_enable(
>> + bsp_priv->clk_mac_refout);
>> + }
>> +
>> + if (!IS_ERR(bsp_priv->aclk_mac))
>> + clk_prepare_enable(bsp_priv->aclk_mac);
>> +
>> + if (!IS_ERR(bsp_priv->pclk_mac))
>> + clk_prepare_enable(bsp_priv->pclk_mac);
>> +
>> + if (!IS_ERR(bsp_priv->mac_clk_tx))
>> + clk_prepare_enable(bsp_priv->mac_clk_tx);
>> +
>> + /**
>> + * if (!IS_ERR(bsp_priv->clk_mac))
>> + * clk_prepare_enable(bsp_priv->clk_mac);
>> + */
>
> why do you need to comment this? could it be related to the stmmaceth
> clk that is used by the main?
>
yes. the same explanation as above
>> + mdelay(5);
>
> hmm, do you actually need this mdelay? Is it to stabilize PLL sources?
>
yes.
>> + bsp_priv->clk_enabled = true;
>> + }
>> + } else {
>> + if (bsp_priv->clk_enabled) {
>> + if (phy_iface == PHY_INTERFACE_MODE_RMII) {
>> + if (!IS_ERR(bsp_priv->mac_clk_rx))
>> + clk_disable_unprepare(
>> + bsp_priv->mac_clk_rx);
>> +
>> + if (!IS_ERR(bsp_priv->clk_mac_ref))
>> + clk_disable_unprepare(
>> + bsp_priv->clk_mac_ref);
>> +
>> + if (!IS_ERR(bsp_priv->clk_mac_refout))
>> + clk_disable_unprepare(
>> + bsp_priv->clk_mac_refout);
>> + }
>> +
>> + if (!IS_ERR(bsp_priv->aclk_mac))
>> + clk_disable_unprepare(bsp_priv->aclk_mac);
>> +
>> + if (!IS_ERR(bsp_priv->pclk_mac))
>> + clk_disable_unprepare(bsp_priv->pclk_mac);
>> +
>> + if (!IS_ERR(bsp_priv->mac_clk_tx))
>> + clk_disable_unprepare(bsp_priv->mac_clk_tx);
>> + /**
>> + * if (!IS_ERR(bsp_priv->clk_mac))
>> + * clk_disable_unprepare(bsp_priv->clk_mac);
>> + */
>> + bsp_priv->clk_enabled = false;
>> + }
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int power_on_by_pmu(struct rk_priv_data *bsp_priv, bool enable)
>> +{
>> + struct regulator *ldo;
>> + char *ldostr = bsp_priv->pmu_regulator;
>> + int ret;
>> +
>> + if (!ldostr) {
>> + pr_err("%s: no ldo found\n", __func__);
>> + return -1;
>> + }
>> +
>> + ldo = regulator_get(NULL, ldostr);
>> + if (!ldo) {
>> + pr_err("\n%s get ldo %s failed\n", __func__, ldostr);
>> + } else {
>> + if (enable) {
>> + if (!regulator_is_enabled(ldo)) {
>> + regulator_set_voltage(ldo, 3300000, 3300000);
>> + ret = regulator_enable(ldo);
>> + if (ret != 0)
>> + pr_err("%s: fail to enable %s\n",
>> + __func__, ldostr);
>> + else
>> + pr_info("turn on ldo done.\n");
>> + } else {
>> + pr_warn("%s is enabled before enable", ldostr);
>> + }
>> + } else {
>> + if (regulator_is_enabled(ldo)) {
>> + ret = regulator_disable(ldo);
>> + if (ret != 0)
>> + pr_err("%s: fail to disable %s\n",
>> + __func__, ldostr);
>> + else
>> + pr_info("turn off ldo done.\n");
>> + } else {
>> + pr_warn("%s is disabled before disable",
>> + ldostr);
>> + }
>> + }
>> + regulator_put(ldo);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int power_on_by_gpio(struct rk_priv_data *bsp_priv, bool enable)
>> +{
>> + if (enable) {
>> + /*power on*/
>> + if (gpio_is_valid(bsp_priv->power_io))
>> + gpio_direction_output(bsp_priv->power_io,
>> + bsp_priv->power_io_level);
>> + } else {
>> + /*power off*/
>> + if (gpio_is_valid(bsp_priv->power_io))
>> + gpio_direction_output(bsp_priv->power_io,
>> + !bsp_priv->power_io_level);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int phy_power_on(struct rk_priv_data *bsp_priv, bool enable)
>> +{
>> + int ret = -1;
>> +
>> + pr_info("Ethernet PHY power %s\n", enable == 1 ? "on" : "off");
>> +
>> + if (bsp_priv->power_ctrl_by_pmu)
>> + ret = power_on_by_pmu(bsp_priv, enable);
>> + else
>> + ret = power_on_by_gpio(bsp_priv, enable);
>> +
>> + if (enable) {
>> + /*reset*/
>> + if (gpio_is_valid(bsp_priv->reset_io)) {
>> + gpio_direction_output(bsp_priv->reset_io,
>> + bsp_priv->reset_io_level);
>> + mdelay(5);
>> + gpio_direction_output(bsp_priv->reset_io,
>> + !bsp_priv->reset_io_level);
>> + }
>> + mdelay(30);
>> +
>> + } else {
>> + /*pull down reset*/
>> + if (gpio_is_valid(bsp_priv->reset_io)) {
>> + gpio_direction_output(bsp_priv->reset_io,
>> + bsp_priv->reset_io_level);
>> + }
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +#define GPIO_PHY_POWER "gmac_phy_power"
>> +#define GPIO_PHY_RESET "gmac_phy_reset"
>> +#define GPIO_PHY_IRQ "gmac_phy_irq"
>> +
>> +static void *rk_gmac_setup(struct platform_device *pdev)
>> +{
>> + struct rk_priv_data *bsp_priv;
>> + struct device *dev = &pdev->dev;
>> + enum of_gpio_flags flags;
>> + int ret;
>> + const char *strings = NULL;
>> + int value;
>> + int irq;
>> +
>> + bsp_priv = devm_kzalloc(dev, sizeof(*bsp_priv), GFP_KERNEL);
>> + if (!bsp_priv)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + bsp_priv->phy_iface = of_get_phy_mode(dev->of_node);
>> +
>> + ret = of_property_read_string(dev->of_node, "pmu_regulator",
>> &strings);
>> + if (ret) {
>> + pr_err("%s: Can not read property: pmu_regulator.\n",
>> __func__);
>> + bsp_priv->power_ctrl_by_pmu = false;
>> + } else {
>> + pr_info("%s: ethernet phy power controlled by pmu(%s).\n",
>> + __func__, strings);
>> + bsp_priv->power_ctrl_by_pmu = true;
>> + strcpy(bsp_priv->pmu_regulator, strings);
>> + }
>> +
>> + ret = of_property_read_u32(dev->of_node, "pmu_enable_level",
>> &value);
>> + if (ret) {
>> + pr_err("%s: Can not read property: pmu_enable_level.\n",
>> + __func__);
>> + bsp_priv->power_ctrl_by_pmu = false;
>> + } else {
>> + pr_info("%s: PHY power controlled by pmu(level = %s).\n",
>> + __func__, (value == 1) ? "HIGH" : "LOW");
>> + bsp_priv->power_ctrl_by_pmu = true;
>> + bsp_priv->pmu_enable_level = value;
>> + }
>> +
>> + ret = of_property_read_string(dev->of_node, "clock_in_out",
>> &strings);
>> + if (ret) {
>> + pr_err("%s: Can not read property: clock_in_out.\n", __func__);
>> + bsp_priv->clock_input = true;
>> + } else {
>> + pr_info("%s: clock input or output? (%s).\n",
>> + __func__, strings);
>> + if (!strcmp(strings, "input"))
>> + bsp_priv->clock_input = true;
>> + else
>> + bsp_priv->clock_input = false;
>> + }
>> +
>> + ret = of_property_read_u32(dev->of_node, "tx_delay", &value);
>> + if (ret) {
>> + bsp_priv->tx_delay = 0x30;
>> + pr_err("%s: Can not read property: tx_delay.", __func__);
>> + pr_err("%s: set tx_delay to 0x%x\n",
>> + __func__, bsp_priv->tx_delay);
>> + } else {
>> + pr_info("%s: TX delay(0x%x).\n", __func__, value);
>> + bsp_priv->tx_delay = value;
>> + }
>> +
>> + ret = of_property_read_u32(dev->of_node, "rx_delay", &value);
>> + if (ret) {
>> + bsp_priv->rx_delay = 0x10;
>> + pr_err("%s: Can not read property: rx_delay.", __func__);
>> + pr_err("%s: set rx_delay to 0x%x\n",
>> + __func__, bsp_priv->rx_delay);
>> + } else {
>> + pr_info("%s: RX delay(0x%x).\n", __func__, value);
>> + bsp_priv->rx_delay = value;
>> + }
>> +
>> + bsp_priv->grf = syscon_regmap_lookup_by_phandle(dev->of_node,
>> + "rockchip,grf");
>> + if (IS_ERR(bsp_priv->grf))
>> + dev_err(&pdev->dev, "Missing rockchip,grf property\n");
>
> I wonder if you can fail on here and save all the check in
> set_rgmii_speed etc.
> Maybe this can be considered a mandatory property for the glue-logic.
>
about fail check, you mean "if(IS_ERR(bsp_priv->grf))" ?
or all of the return value of of_property_*
>> +
>> + bsp_priv->phyirq_io =
>> + of_get_named_gpio_flags(dev->of_node,
>> + "phyirq-gpio", 0, &flags);
>> + bsp_priv->phyirq_io_level = (flags & OF_GPIO_ACTIVE_LOW) ? 0 : 1;
>
>
> Sorry, did you document all in Documentation/devicetree/bindings/net/ ?
>
>> +
>> + bsp_priv->reset_io =
>> + of_get_named_gpio_flags(dev->of_node,
>> + "reset-gpio", 0, &flags);
>> + bsp_priv->reset_io_level = (flags & OF_GPIO_ACTIVE_LOW) ? 0 : 1;
>> +
>> + bsp_priv->power_io =
>> + of_get_named_gpio_flags(dev->of_node, "power-gpio", 0, &flags);
>> + bsp_priv->power_io_level = (flags & OF_GPIO_ACTIVE_LOW) ? 0 : 1;
>> +
>> + /*power*/
>> + if (!gpio_is_valid(bsp_priv->power_io)) {
>> + pr_err("%s: Failed to get GPIO %s.\n",
>> + __func__, "power-gpio");
>> + } else {
>> + ret = gpio_request(bsp_priv->power_io, GPIO_PHY_POWER);
>> + if (ret)
>> + pr_err("%s: ERROR: Failed to request GPIO %s.\n",
>> + __func__, GPIO_PHY_POWER);
>> + }
>> +
>> + if (!gpio_is_valid(bsp_priv->reset_io)) {
>> + pr_err("%s: ERROR: Get reset-gpio failed.\n", __func__);
>> + } else {
>> + ret = gpio_request(bsp_priv->reset_io, GPIO_PHY_RESET);
>> + if (ret)
>> + pr_err("%s: ERROR: Failed to request GPIO %s.\n",
>> + __func__, GPIO_PHY_RESET);
>> + }
>> +
>> + if (bsp_priv->phyirq_io > 0) {
>> + struct plat_stmmacenet_data *plat_dat;
>> +
>> + pr_info("PHY irq in use\n");
>> + ret = gpio_request(bsp_priv->phyirq_io, GPIO_PHY_IRQ);
>> + if (ret < 0) {
>> + pr_warn("%s: Failed to request GPIO %s\n",
>> + __func__, GPIO_PHY_IRQ);
>> + goto goon;
>> + }
>> +
>> + ret = gpio_direction_input(bsp_priv->phyirq_io);
>> + if (ret < 0) {
>> + pr_err("%s, Failed to set input for GPIO %s\n",
>> + __func__, GPIO_PHY_IRQ);
>> + gpio_free(bsp_priv->phyirq_io);
>> + goto goon;
>> + }
>> +
>> + irq = gpio_to_irq(bsp_priv->phyirq_io);
>> + if (irq < 0) {
>> + ret = irq;
>> + pr_err("Failed to set irq for %s\n",
>> + GPIO_PHY_IRQ);
>> + gpio_free(bsp_priv->phyirq_io);
>> + goto goon;
>> + }
>> +
>> + plat_dat = dev_get_platdata(&pdev->dev);
>> + if (plat_dat)
>> + plat_dat->mdio_bus_data->probed_phy_irq = irq;
>> + else
>> + pr_err("%s: plat_data is NULL\n", __func__);
>> + }
>> +
>> +goon:
>> + /*rmii or rgmii*/
>> + if (bsp_priv->phy_iface == PHY_INTERFACE_MODE_RGMII) {
>> + pr_info("%s: init for RGMII\n", __func__);
>> + set_to_rgmii(bsp_priv, bsp_priv->tx_delay, bsp_priv->rx_delay);
>> + } else if (bsp_priv->phy_iface == PHY_INTERFACE_MODE_RMII) {
>> + pr_info("%s: init for RMII\n", __func__);
>> + set_to_rmii(bsp_priv);
>> + } else {
>> + pr_err("%s: ERROR: NO interface defined!\n", __func__);
>> + }
>> +
>> + bsp_priv->pdev = pdev;
>> +
>> + gmac_clk_init(bsp_priv);
>> +
>> + return bsp_priv;
>> +}
>> +
>> +static int rk_gmac_init(struct platform_device *pdev, void *priv)
>> +{
>> + struct rk_priv_data *bsp_priv = priv;
>> + int ret;
>> +
>> + ret = phy_power_on(bsp_priv, true);
>> + if (ret)
>> + return ret;
>> +
>> + ret = gmac_clk_enable(bsp_priv, true);
>> + if (ret)
>> + return ret;
>> +
>> + return 0;
>> +}
>> +
>> +static void rk_gmac_exit(struct platform_device *pdev, void *priv)
>> +{
>> + struct rk_priv_data *gmac = priv;
>> +
>> + phy_power_on(gmac, false);
>> + gmac_clk_enable(gmac, false);
>> +}
>> +
>> +static void rk_fix_speed(void *priv, unsigned int speed)
>> +{
>> + struct rk_priv_data *bsp_priv = priv;
>> +
>> + if (bsp_priv->phy_iface == PHY_INTERFACE_MODE_RGMII)
>> + set_rgmii_speed(bsp_priv, speed);
>> + else if (bsp_priv->phy_iface == PHY_INTERFACE_MODE_RMII)
>> + set_rmii_speed(bsp_priv, speed);
>> + else
>> + pr_err("unsupported interface %d", bsp_priv->phy_iface);
>> +}
>> +
>> +const struct stmmac_of_data rk_gmac_data = {
>> + .has_gmac = 1,
>> + .tx_coe = 1,
>
> FYI, on new gmac there is the HW capability register to dinamically
> provide you if coe is supported.
>
> IMO you should add the OF "compatible" string and in case of mac
> newer than the 3.50a you can remove coe.
>
>
> BR
> Peppe
>
>> + .fix_mac_speed = rk_fix_speed,
>> + .setup = rk_gmac_setup,
>> + .init = rk_gmac_init,
>> + .exit = rk_gmac_exit,
>> +};
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>> b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>> index 15814b7..b4dee96 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>> @@ -33,6 +33,7 @@
>>
>> static const struct of_device_id stmmac_dt_ids[] = {
>> /* SoC specific glue layers should come before generic bindings */
>> + { .compatible = "rockchip,rk3288-gmac", .data = &rk_gmac_data},
>> { .compatible = "amlogic,meson6-dwmac", .data =
>> &meson6_dwmac_data},
>> { .compatible = "allwinner,sun7i-a20-gmac", .data =
>> &sun7i_gmac_data},
>> { .compatible = "st,stih415-dwmac", .data = &stih4xx_dwmac_data},
>> @@ -291,6 +292,8 @@ static int stmmac_pltfr_probe(struct
>> platform_device *pdev)
>> return -ENOMEM;
>> }
>>
>> + pdev->dev.platform_data = plat_dat;
>> +
>> ret = stmmac_probe_config_dt(pdev, plat_dat, &mac);
>> if (ret) {
>> pr_err("%s: main dt probe failed", __func__);
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.h
>> b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.h
>> index 25dd1f7..32a0516 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.h
>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.h
>> @@ -24,5 +24,6 @@ extern const struct stmmac_of_data sun7i_gmac_data;
>> extern const struct stmmac_of_data stih4xx_dwmac_data;
>> extern const struct stmmac_of_data stid127_dwmac_data;
>> extern const struct stmmac_of_data socfpga_gmac_data;
>> +extern const struct stmmac_of_data rk_gmac_data;
>>
>> #endif /* __STMMAC_PLATFORM_H__ */
>>
>
>
>
>
^ permalink raw reply
* [PATCH] xen-netfront: Remove BUGs on paged skb data which crosses a page boundary
From: Seth Forshee @ 2014-11-26 2:28 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk, Boris Ostrovsky, David Vrabel
Cc: Zoltan Kiss, Eric Dumazet, Stefan Bader, Seth Forshee, xen-devel,
netdev, linux-kernel
These BUGs can be erroneously triggered by frags which refer to
tail pages within a compound page. The data in these pages may
overrun the hardware page while still being contained within the
compound page, but since compound_order() evaluates to 0 for tail
pages the assertion fails. The code already iterates through
subsequent pages correctly in this scenario, so the BUGs are
unnecessary and can be removed.
Fixes: f36c374782e4 ("xen/netfront: handle compound page fragments on transmit")
Cc: <stable@vger.kernel.org> # 3.7+
Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
drivers/net/xen-netfront.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index cca871346a0f..ece8d1804d13 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -496,9 +496,6 @@ static void xennet_make_frags(struct sk_buff *skb, struct netfront_queue *queue,
len = skb_frag_size(frag);
offset = frag->page_offset;
- /* Data must not cross a page boundary. */
- BUG_ON(len + offset > PAGE_SIZE<<compound_order(page));
-
/* Skip unused frames from start of page */
page += offset >> PAGE_SHIFT;
offset &= ~PAGE_MASK;
@@ -506,8 +503,6 @@ static void xennet_make_frags(struct sk_buff *skb, struct netfront_queue *queue,
while (len > 0) {
unsigned long bytes;
- BUG_ON(offset >= PAGE_SIZE);
-
bytes = PAGE_SIZE - offset;
if (bytes > len)
bytes = len;
--
1.9.1
^ permalink raw reply related
* [PATCH V3 1/1] ipv6: Remove unnecessary test
From: Zhu Yanjun @ 2014-11-26 2:25 UTC (permalink / raw)
To: zyjzyj2000, netdev, davem
Cc: Zhu Yanjun, Hong Zhiguo, Octavian Purdila, Pavel Emelyanov,
Cong Wang
The "init_net" test in function addrconf_exit_net is introduced
in commit 44a6bd29 [Create ipv6 devconf-s for namespaces] to avoid freeing
init_net. In commit c900a800 [ipv6: fix bad free of addrconf_init_net],
function addrconf_init_net will allocate memory for every net regardless of
init_net. In this case, it is unnecessary to make "init_net" test.
CC: Hong Zhiguo <honkiko@gmail.com>
CC: Octavian Purdila <opurdila@ixiacom.com>
CC: Pavel Emelyanov <xemul@openvz.org>
CC: Cong Wang <cwang@twopensource.com>
Suggested-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Zhu Yanjun <Yanjun.Zhu@windriver.com>
---
net/ipv6/addrconf.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 0169ccf..b8724fc 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -5336,10 +5336,8 @@ static void __net_exit addrconf_exit_net(struct net *net)
__addrconf_sysctl_unregister(net->ipv6.devconf_dflt);
__addrconf_sysctl_unregister(net->ipv6.devconf_all);
#endif
- if (!net_eq(net, &init_net)) {
- kfree(net->ipv6.devconf_dflt);
- kfree(net->ipv6.devconf_all);
- }
+ kfree(net->ipv6.devconf_dflt);
+ kfree(net->ipv6.devconf_all);
}
static struct pernet_operations addrconf_ops = {
--
1.9.1
^ permalink raw reply related
* [PATCH V2 1/1] ipv6: Remove unnecessary test
From: Zhu Yanjun @ 2014-11-26 2:23 UTC (permalink / raw)
To: zyjzyj2000, netdev, davem
Cc: Zhu Yanjun, Hong Zhiguo, Octavian Purdila, Pavel Emelyanov,
Cong Wang
The "init_net" condition judgement in function addrconf_exit_net is introduced
in commit 44a6bd29 [Create ipv6 devconf-s for namespaces] to avoid freeing
init_net. In commit c900a800 [ipv6: fix bad free of addrconf_init_net],
function addrconf_init_net will allocate memory for every net regardless of
init_net. In this case, it is unnecessary to make "init_net" condition judgement.
CC: Hong Zhiguo <honkiko@gmail.com>
CC: Octavian Purdila <opurdila@ixiacom.com>
CC: Pavel Emelyanov <xemul@openvz.org>
CC: Cong Wang <cwang@twopensource.com>
Suggested-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Zhu Yanjun <Yanjun.Zhu@windriver.com>
---
net/ipv6/addrconf.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 0169ccf..b8724fc 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -5336,10 +5336,8 @@ static void __net_exit addrconf_exit_net(struct net *net)
__addrconf_sysctl_unregister(net->ipv6.devconf_dflt);
__addrconf_sysctl_unregister(net->ipv6.devconf_all);
#endif
- if (!net_eq(net, &init_net)) {
- kfree(net->ipv6.devconf_dflt);
- kfree(net->ipv6.devconf_all);
- }
+ kfree(net->ipv6.devconf_dflt);
+ kfree(net->ipv6.devconf_all);
}
static struct pernet_operations addrconf_ops = {
--
1.9.1
^ permalink raw reply related
* [PATCH net 2/2] net: dsa: bcm_sf2: reset switch prior to initialization
From: Florian Fainelli @ 2014-11-26 2:08 UTC (permalink / raw)
To: netdev; +Cc: davem, Florian Fainelli
In-Reply-To: <1416967729-717-1-git-send-email-f.fainelli@gmail.com>
Our boot agent may have left the switch in an certain configuration
state, make sure we issue a software reset prior to configuring the
switch in order to ensure the HW is in a consistent state, in particular
transmit queues and internal buffers.
Fixes: 246d7f773c13 ("net: dsa: add Broadcom SF2 switch driver")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/dsa/bcm_sf2.c | 52 ++++++++++++++++++++++++++---------------------
1 file changed, 29 insertions(+), 23 deletions(-)
diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index 46632e8b6336..4f4c2a7888e5 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -377,6 +377,29 @@ static irqreturn_t bcm_sf2_switch_1_isr(int irq, void *dev_id)
return IRQ_HANDLED;
}
+static int bcm_sf2_sw_rst(struct bcm_sf2_priv *priv)
+{
+ unsigned int timeout = 1000;
+ u32 reg;
+
+ reg = core_readl(priv, CORE_WATCHDOG_CTRL);
+ reg |= SOFTWARE_RESET | EN_CHIP_RST | EN_SW_RESET;
+ core_writel(priv, reg, CORE_WATCHDOG_CTRL);
+
+ do {
+ reg = core_readl(priv, CORE_WATCHDOG_CTRL);
+ if (!(reg & SOFTWARE_RESET))
+ break;
+
+ usleep_range(1000, 2000);
+ } while (timeout-- > 0);
+
+ if (timeout == 0)
+ return -ETIMEDOUT;
+
+ return 0;
+}
+
static int bcm_sf2_sw_setup(struct dsa_switch *ds)
{
const char *reg_names[BCM_SF2_REGS_NUM] = BCM_SF2_REGS_NAME;
@@ -410,6 +433,12 @@ static int bcm_sf2_sw_setup(struct dsa_switch *ds)
base++;
}
+ ret = bcm_sf2_sw_rst(priv);
+ if (ret) {
+ pr_err("unable to software reset switch: %d\n", ret);
+ goto out_unmap;
+ }
+
/* Disable all interrupts and request them */
intrl2_0_writel(priv, 0xffffffff, INTRL2_CPU_MASK_SET);
intrl2_0_writel(priv, 0xffffffff, INTRL2_CPU_CLEAR);
@@ -735,29 +764,6 @@ static int bcm_sf2_sw_suspend(struct dsa_switch *ds)
return 0;
}
-static int bcm_sf2_sw_rst(struct bcm_sf2_priv *priv)
-{
- unsigned int timeout = 1000;
- u32 reg;
-
- reg = core_readl(priv, CORE_WATCHDOG_CTRL);
- reg |= SOFTWARE_RESET | EN_CHIP_RST | EN_SW_RESET;
- core_writel(priv, reg, CORE_WATCHDOG_CTRL);
-
- do {
- reg = core_readl(priv, CORE_WATCHDOG_CTRL);
- if (!(reg & SOFTWARE_RESET))
- break;
-
- usleep_range(1000, 2000);
- } while (timeout-- > 0);
-
- if (timeout == 0)
- return -ETIMEDOUT;
-
- return 0;
-}
-
static int bcm_sf2_sw_resume(struct dsa_switch *ds)
{
struct bcm_sf2_priv *priv = ds_to_priv(ds);
--
2.1.0
^ permalink raw reply related
* [PATCH net 1/2] net: dsa: bcm_sf2: fix unmapping registers in case of errors
From: Florian Fainelli @ 2014-11-26 2:08 UTC (permalink / raw)
To: netdev; +Cc: davem, Florian Fainelli
In-Reply-To: <1416967729-717-1-git-send-email-f.fainelli@gmail.com>
In case we fail to ioremap() one of our registers, we would be leaking
existing mappings, unwind those accordingly on errors.
Fixes: 246d7f773c13 ("net: dsa: add Broadcom SF2 switch driver")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/dsa/bcm_sf2.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index b9625968daac..46632e8b6336 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -404,7 +404,8 @@ static int bcm_sf2_sw_setup(struct dsa_switch *ds)
*base = of_iomap(dn, i);
if (*base == NULL) {
pr_err("unable to find register: %s\n", reg_names[i]);
- return -ENODEV;
+ ret = -ENOMEM;
+ goto out_unmap;
}
base++;
}
@@ -484,7 +485,8 @@ out_free_irq0:
out_unmap:
base = &priv->core;
for (i = 0; i < BCM_SF2_REGS_NUM; i++) {
- iounmap(*base);
+ if (*base)
+ iounmap(*base);
base++;
}
return ret;
--
2.1.0
^ permalink raw reply related
* [PATCH net 0/2] net: dsa: bcm_sf2: misc bugfixes
From: Florian Fainelli @ 2014-11-26 2:08 UTC (permalink / raw)
To: netdev; +Cc: davem, Florian Fainelli
Hi David,
This patch series contains two bug fixes:
- first patch fixes an issue on the error path of the driver where we could
have left some of our registers mapped
- second patch enforces the use of a software reset of the switch to guarantee
the HW is in a consistent state prior to software initialization
Thanks!
Florian Fainelli (2):
net: dsa: bcm_sf2: fix unmapping registers in case of errors
net: dsa: bcm_sf2: reset switch prior to initialization
drivers/net/dsa/bcm_sf2.c | 58 +++++++++++++++++++++++++++--------------------
1 file changed, 33 insertions(+), 25 deletions(-)
--
2.1.0
^ permalink raw reply
* [PATCH net 0/2] net: dsa: bcm_sf2: misc bugfixes
From: Florian Fainelli @ 2014-11-26 2:08 UTC (permalink / raw)
To: netdev; +Cc: davem, Florian Fainelli
Hi David,
This patch series contains two bug fixes:
- first patch fixes an issue on the error path of the driver where we could
have left some of our registers mapped
- second patch enforces the use of a software reset of the switch to guarantee
the HW is in a consistent state prior to software initialization
Thanks!
Florian Fainelli (2):
net: dsa: bcm_sf2: fix unmapping registers in case of errors
net: dsa: bcm_sf2: reset switch prior to initialization
drivers/net/dsa/bcm_sf2.c | 58 +++++++++++++++++++++++++++--------------------
1 file changed, 33 insertions(+), 25 deletions(-)
--
2.1.0
^ permalink raw reply
* Re: [patch net-next v3 09/17] bridge: add API to notify bridge driver of learned FBD on offloaded device
From: Scott Feldman @ 2014-11-26 2:03 UTC (permalink / raw)
To: Florian Fainelli
Cc: Jiri Pirko, Netdev, David S. Miller, nhorman@tuxdriver.com,
Andy Gospodarek, Thomas Graf, dborkman@redhat.com,
ogerlitz@mellanox.com, jesse@nicira.com, pshelar@nicira.com,
azhou@nicira.com, ben@decadent.org.uk, stephen@networkplumber.org,
Kirsher, Jeffrey T, vyasevic@redhat.com, Cong Wang,
Fastabend, John R, Eric Dumazet, Jamal Hadi Salim, Roopa Prabhu,
John Linville
In-Reply-To: <54750669.4090406@gmail.com>
On Tue, Nov 25, 2014 at 12:44 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> On 25/11/14 02:28, Jiri Pirko wrote:
>> From: Scott Feldman <sfeldma@gmail.com>
>>
>> When the swdev device learns a new mac/vlan on a port, it sends some async
>> notification to the driver and the driver installs an FDB in the device.
>> To give a holistic system view, the learned mac/vlan should be reflected
>> in the bridge's FBD table, so the user, using normal iproute2 cmds, can view
>> what is currently learned by the device. This API on the bridge driver gives
>> a way for the swdev driver to install an FBD entry in the bridge FBD table.
>> (And remove one).
>>
>> This is equivalent to the device running these cmds:
>>
>> bridge fdb [add|del] <mac> dev <dev> vid <vlan id> master
>>
>> This patch needs some extra eyeballs for review, in paricular around the
>> locking and contexts.
>>
>> Signed-off-by: Scott Feldman <sfeldma@gmail.com>
>> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>> ---
>
> [snip]
>
>> + head = &br->hash[br_mac_hash(addr, vid)];
>> + fdb = fdb_find(head, addr, vid);
>> + if (!fdb) {
>> + fdb = fdb_create(head, p, addr, vid);
>> + if (!fdb) {
>> + err = -ENOMEM;
>> + goto err_unlock;
>> + }
>> + fdb->added_by_external_learn = 1;
>> + fdb_notify(br, fdb, RTM_NEWNEIGH);
>> + } else if (fdb->added_by_external_learn) {
>> + /* Refresh entry */
>> + fdb->updated = fdb->used = jiffies;
>> + } else if (!fdb->added_by_user) {
>> + /* Take over SW learned entry */
>> + fdb->added_by_external_learn = 1;
>> + fdb->updated = jiffies;
>> + fdb_notify(br, fdb, RTM_NEWNEIGH);
>> + }
>
> Is there any case where this fdb entry gets re-used and is no longer
> added by an external learning? Should we clear this flag somewhere?
Once the FDB entry is marked "added_by_external_learn" it stays marked
as such until removed by aging cleanup process (or flushed due to
interface down, etc). If aged out (and now deleted), the FDB entry
may come back either by SW learn or by HW learn. If SW learn comes
first, and then HW learn, HW learn will override and mark the existing
FDB entry "added_by_external_learn". So there is take-over by HW but
no give-back to SW. And there is no explicit clearing of the mark
short of deleting the FDB entry. The mark is mostly for letting
user's know which FDB entries where learned by HW and synced to the
bridge's FDB.
> [snip]
>
>> +EXPORT_SYMBOL(br_fdb_external_learn_del);
>> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
>> index 4f577c4..02cd63b 100644
>> --- a/net/bridge/br_private.h
>> +++ b/net/bridge/br_private.h
>> @@ -101,6 +101,7 @@ struct net_bridge_fdb_entry
>> unsigned char is_local;
>> unsigned char is_static;
>> unsigned char added_by_user;
>> + unsigned char added_by_external_learn;
>
> Pheww, we could be saving some memory footprint here by using different
> types here ;)
> --
> Florian
^ permalink raw reply
* Re: [patch net-next v3 09/17] bridge: add API to notify bridge driver of learned FBD on offloaded device
From: Scott Feldman @ 2014-11-26 1:48 UTC (permalink / raw)
To: Thomas Graf
Cc: Andy Gospodarek, Jiri Pirko, Netdev, David S. Miller,
nhorman@tuxdriver.com, Andy Gospodarek, dborkman@redhat.com,
ogerlitz@mellanox.com, jesse@nicira.com, pshelar@nicira.com,
azhou@nicira.com, ben@decadent.org.uk, stephen@networkplumber.org,
Kirsher, Jeffrey T, vyasevic@redhat.com, Cong Wang,
Fastabend, John R, Eric Dumazet, Jamal Hadi Salim,
Florian Fainelli, Roopa Prabhu
In-Reply-To: <20141125223643.GH3912@casper.infradead.org>
On Tue, Nov 25, 2014 at 12:36 PM, Thomas Graf <tgraf@suug.ch> wrote:
> On 11/25/14 at 11:38am, Andy Gospodarek wrote:
>> On Tue, Nov 25, 2014 at 11:28:40AM +0100, Jiri Pirko wrote:
>> > From: Scott Feldman <sfeldma@gmail.com>
>> >
>> > When the swdev device learns a new mac/vlan on a port, it sends some async
>> > notification to the driver and the driver installs an FDB in the device.
>> > To give a holistic system view, the learned mac/vlan should be reflected
>> > in the bridge's FBD table, so the user, using normal iproute2 cmds, can view
>> > what is currently learned by the device. This API on the bridge driver gives
>> > a way for the swdev driver to install an FBD entry in the bridge FBD table.
>> > (And remove one).
>> >
>> > This is equivalent to the device running these cmds:
>> >
>> > bridge fdb [add|del] <mac> dev <dev> vid <vlan id> master
>> >
>> > This patch needs some extra eyeballs for review, in paricular around the
>> > locking and contexts.
>> >
>> > Signed-off-by: Scott Feldman <sfeldma@gmail.com>
>> > Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>
> I like the simplicity of this. That said, given we'll have multiple
> users of swdev including OVS, shouldn't this be a notifier or a
> callback that depends on who is controlling the device?
I like the idea. When the switch port joins Linux bridge or OVS
datapath, a callback is registered with the driver. That way the
driver doesn't really care if the port is a bridge member or an OVS
vport in a datapath. It's just passing up the FDB entry
(port/mac/vlan) details to the container device. Can we hold this
idea until this patchset sticks? I think once OVS support comes back
into the swdev model would be the time to address this.
>
>> > + spin_lock(&br->hash_lock);
>> (Since you asked to check locking...)
>>
>> Most of the other fdb_add/delete/insert/update calls take this with
>> spin_lock_bh. Did you try this with lockdep enabled just to see if that
>> is needed here? I suspect that anytime br->hash_lock is taken it will
>> need to be with softirqs disabled from this point forward.
>
> At least br_fdb_update() seems to be called from BH context so I would
> agree and argue the lock in br_fdb_cleanup() and br_fdb_update() need a
> fix too. I'll send a patch.
^ permalink raw reply
* Re: [patch net-next v3 02/17] net: make vid as a parameter for ndo_fdb_add/ndo_fdb_del
From: Simon Horman @ 2014-11-26 1:44 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: John Fastabend, Jiri Pirko, netdev, davem, nhorman, andy, tgraf,
dborkman, ogerlitz, jesse, pshelar, azhou, ben, stephen,
jeffrey.t.kirsher, vyasevic, xiyou.wangcong, edumazet, sfeldma,
f.fainelli, roopa, linville, jasowang, ebiederm, nicolas.dichtel,
ryazanov.s.a, buytenh, aviadr, nbd, alexei.starovoitov,
Neil.Jerram, ronye, alexander.h.duyck, john.ronciak, mleitner,
shrijeet, gospo, bcrl
In-Reply-To: <5474B353.10802@mojatatu.com>
On Tue, Nov 25, 2014 at 11:50:27AM -0500, Jamal Hadi Salim wrote:
> On 11/25/14 11:30, John Fastabend wrote:
> >On 11/25/2014 08:18 AM, Jamal Hadi Salim wrote:
> >>On 11/25/14 11:01, John Fastabend wrote:
> >>>On 11/25/2014 07:38 AM, Jamal Hadi Salim wrote:
> >>>>On 11/25/14 05:28, Jiri Pirko wrote:
> >>>>>Do the work of parsing NDA_VLAN directly in rtnetlink code, pass simple
> >>>>>u16 vid to drivers from there.
> >>>>>
> >>>>
>
>
> >Actually (after having some coffee) this becomes much more useful
> >if you return which items failed. Then you can slam the hardware
> >with your 100 entries, probably a lot more then that, and come back
> >later and clean it up.
> >
>
> Yes, that is the general use case.
> Unfortunately at the moment we only return codes on a netlink set
> direction - but would be a beauty if we could return what succeeded
> and didnt in some form of vector.
> Note: all is not lost because you can always do a get afterwards and
> find what is missing if you got a return code of "partial success".
> Just a little less efficient..
I agree entirely. But efficiency may be a very real issue in practice.
> >We return a bitmask of which operations were successful. So if SW fails
> >we have both bits cleared and we abort. When SW is successful we set the
> >SW bit and try to program the HW. If its sucessful we set the HW bit if
> >its not we abort with an err. Converting this to (1) is not much work
> >just skip the abort.
> >
>
> Ok, guess i am gonna have to go stare at the code some more.
> I thought we returned one of the error codes?
> A bitmask would work for a single entry - because you have two
> options add to h/ware and/or s/ware. So response is easy to encode.
> But if i have 1000 and they are sparsely populated (think an indexed
> table and i have indices 1, 23, 45, etc), then a bitmask would be
> hard to use.
>
> cheers,
> jamal
^ permalink raw reply
* Re: [net-next PATCH 2/5] ethernet/intel: Use eth_skb_pad helper
From: Eric Dumazet @ 2014-11-26 1:43 UTC (permalink / raw)
To: Alexander Duyck; +Cc: Alexander Duyck, netdev, Jeff Kirsher, davem
In-Reply-To: <54752263.2040204@gmail.com>
On Tue, 2014-11-25 at 16:44 -0800, Alexander Duyck wrote:
>
> I wonder if we couldn't make this some sort of netdevice attribute to
> indicate what the smallest frame we can handle is and then just pad the
> frame to that as a part of __dev_xmit_skb. Then we could do that
> outside of the locks and take care of it before we even hit the qdisc layer.
Well, many NIC do not have such restriction.
Do we have some kind of counters of skb->head reallocations caused by
this padding ?
I believe I finally have an idea why we had various + 15 in skb
allocations in TCP stack !
We probably should reinstate them, as ACK packets can be 54 bytes long.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox