* [net-next 1/3] tipc: support broadcast/replicast configurable for bc-link
From: Hoang Le @ 2019-02-19 11:30 UTC (permalink / raw)
To: jon.maloy, maloy, ying.xue, netdev, tipc-discussion
Currently, a multicast stream uses either broadcast or replicast as
transmission method, based on the ratio between number of actual
destinations nodes and cluster size.
However, when an L2 interface (e.g., VXLAN) provides pseudo
broadcast support, this becomes very inefficient, as it blindly
replicates multicast packets to all cluster/subnet nodes,
irrespective of whether they host actual target sockets or not.
The TIPC multicast algorithm is able to distinguish real destination
nodes from other nodes, and hence provides a smarter and more
efficient method for transferring multicast messages than
pseudo broadcast can do.
Because of this, we now make it possible for users to force
the broadcast link to permanently switch to using replicast,
irrespective of which capabilities the bearer provides,
or pretend to provide.
Conversely, we also make it possible to force the broadcast link
to always use true broadcast. While maybe less useful in
deployed systems, this may at least be useful for testing the
broadcast algorithm in small clusters.
We retain the current AUTOSELECT ability, i.e., to let the broadcast link
automatically select which algorithm to use, and to switch back and forth
between broadcast and replicast as the ratio between destination
node number and cluster size changes. This remains the default method.
Furthermore, we make it possible to configure the threshold ratio for
such switches. The default ratio is now set to 10%, down from 25% in the
earlier implementation.
Acked-by: Jon Maloy <jon.maloy@ericsson.com>
Signed-off-by: Hoang Le <hoang.h.le@dektech.com.au>
---
include/uapi/linux/tipc_netlink.h | 2 +
net/tipc/bcast.c | 104 ++++++++++++++++++++++++++++--
net/tipc/bcast.h | 7 ++
net/tipc/link.c | 8 +++
net/tipc/netlink.c | 4 +-
5 files changed, 120 insertions(+), 5 deletions(-)
diff --git a/include/uapi/linux/tipc_netlink.h b/include/uapi/linux/tipc_netlink.h
index 0ebe02ef1a86..efb958fd167d 100644
--- a/include/uapi/linux/tipc_netlink.h
+++ b/include/uapi/linux/tipc_netlink.h
@@ -281,6 +281,8 @@ enum {
TIPC_NLA_PROP_TOL, /* u32 */
TIPC_NLA_PROP_WIN, /* u32 */
TIPC_NLA_PROP_MTU, /* u32 */
+ TIPC_NLA_PROP_BROADCAST, /* u32 */
+ TIPC_NLA_PROP_BROADCAST_RATIO, /* u32 */
__TIPC_NLA_PROP_MAX,
TIPC_NLA_PROP_MAX = __TIPC_NLA_PROP_MAX - 1
diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c
index d8026543bf4c..12b59268bdd6 100644
--- a/net/tipc/bcast.c
+++ b/net/tipc/bcast.c
@@ -54,7 +54,9 @@ const char tipc_bclink_name[] = "broadcast-link";
* @dests: array keeping number of reachable destinations per bearer
* @primary_bearer: a bearer having links to all broadcast destinations, if any
* @bcast_support: indicates if primary bearer, if any, supports broadcast
+ * @force_bcast: forces broadcast for multicast traffic
* @rcast_support: indicates if all peer nodes support replicast
+ * @force_rcast: forces replicast for multicast traffic
* @rc_ratio: dest count as percentage of cluster size where send method changes
* @bc_threshold: calculated from rc_ratio; if dests > threshold use broadcast
*/
@@ -64,7 +66,9 @@ struct tipc_bc_base {
int dests[MAX_BEARERS];
int primary_bearer;
bool bcast_support;
+ bool force_bcast;
bool rcast_support;
+ bool force_rcast;
int rc_ratio;
int bc_threshold;
};
@@ -485,10 +489,63 @@ static int tipc_bc_link_set_queue_limits(struct net *net, u32 limit)
return 0;
}
+static int tipc_bc_link_set_broadcast_mode(struct net *net, u32 bc_mode)
+{
+ struct tipc_bc_base *bb = tipc_bc_base(net);
+
+ switch (bc_mode) {
+ case BCLINK_MODE_BCAST:
+ if (!bb->bcast_support)
+ return -ENOPROTOOPT;
+
+ bb->force_bcast = true;
+ bb->force_rcast = false;
+ break;
+ case BCLINK_MODE_RCAST:
+ if (!bb->rcast_support)
+ return -ENOPROTOOPT;
+
+ bb->force_bcast = false;
+ bb->force_rcast = true;
+ break;
+ case BCLINK_MODE_SEL:
+ if (!bb->bcast_support || !bb->rcast_support)
+ return -ENOPROTOOPT;
+
+ bb->force_bcast = false;
+ bb->force_rcast = false;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int tipc_bc_link_set_broadcast_ratio(struct net *net, u32 bc_ratio)
+{
+ struct tipc_bc_base *bb = tipc_bc_base(net);
+
+ if (!bb->bcast_support || !bb->rcast_support)
+ return -ENOPROTOOPT;
+
+ if (bc_ratio > 100 || bc_ratio <= 0)
+ return -EINVAL;
+
+ bb->rc_ratio = bc_ratio;
+ tipc_bcast_lock(net);
+ tipc_bcbase_calc_bc_threshold(net);
+ tipc_bcast_unlock(net);
+
+ return 0;
+}
+
int tipc_nl_bc_link_set(struct net *net, struct nlattr *attrs[])
{
int err;
u32 win;
+ u32 bc_mode;
+ u32 bc_ratio;
struct nlattr *props[TIPC_NLA_PROP_MAX + 1];
if (!attrs[TIPC_NLA_LINK_PROP])
@@ -498,12 +555,28 @@ int tipc_nl_bc_link_set(struct net *net, struct nlattr *attrs[])
if (err)
return err;
- if (!props[TIPC_NLA_PROP_WIN])
+ if (!props[TIPC_NLA_PROP_WIN] &&
+ !props[TIPC_NLA_PROP_BROADCAST] &&
+ !props[TIPC_NLA_PROP_BROADCAST_RATIO]) {
return -EOPNOTSUPP;
+ }
+
+ if (props[TIPC_NLA_PROP_BROADCAST]) {
+ bc_mode = nla_get_u32(props[TIPC_NLA_PROP_BROADCAST]);
+ err = tipc_bc_link_set_broadcast_mode(net, bc_mode);
+ }
- win = nla_get_u32(props[TIPC_NLA_PROP_WIN]);
+ if (!err && props[TIPC_NLA_PROP_BROADCAST_RATIO]) {
+ bc_ratio = nla_get_u32(props[TIPC_NLA_PROP_BROADCAST_RATIO]);
+ err = tipc_bc_link_set_broadcast_ratio(net, bc_ratio);
+ }
- return tipc_bc_link_set_queue_limits(net, win);
+ if (!err && props[TIPC_NLA_PROP_WIN]) {
+ win = nla_get_u32(props[TIPC_NLA_PROP_WIN]);
+ err = tipc_bc_link_set_queue_limits(net, win);
+ }
+
+ return err;
}
int tipc_bcast_init(struct net *net)
@@ -529,7 +602,7 @@ int tipc_bcast_init(struct net *net)
goto enomem;
bb->link = l;
tn->bcl = l;
- bb->rc_ratio = 25;
+ bb->rc_ratio = 10;
bb->rcast_support = true;
return 0;
enomem:
@@ -576,3 +649,26 @@ void tipc_nlist_purge(struct tipc_nlist *nl)
nl->remote = 0;
nl->local = false;
}
+
+u32 tipc_bcast_get_broadcast_mode(struct net *net)
+{
+ struct tipc_bc_base *bb = tipc_bc_base(net);
+
+ if (bb->force_bcast)
+ return BCLINK_MODE_BCAST;
+
+ if (bb->force_rcast)
+ return BCLINK_MODE_RCAST;
+
+ if (bb->bcast_support && bb->rcast_support)
+ return BCLINK_MODE_SEL;
+
+ return 0;
+}
+
+u32 tipc_bcast_get_broadcast_ratio(struct net *net)
+{
+ struct tipc_bc_base *bb = tipc_bc_base(net);
+
+ return bb->rc_ratio;
+}
diff --git a/net/tipc/bcast.h b/net/tipc/bcast.h
index 751530ab0c49..37c55e7347a5 100644
--- a/net/tipc/bcast.h
+++ b/net/tipc/bcast.h
@@ -48,6 +48,10 @@ extern const char tipc_bclink_name[];
#define TIPC_METHOD_EXPIRE msecs_to_jiffies(5000)
+#define BCLINK_MODE_BCAST 0x1
+#define BCLINK_MODE_RCAST 0x2
+#define BCLINK_MODE_SEL 0x4
+
struct tipc_nlist {
struct list_head list;
u32 self;
@@ -92,6 +96,9 @@ int tipc_nl_add_bc_link(struct net *net, struct tipc_nl_msg *msg);
int tipc_nl_bc_link_set(struct net *net, struct nlattr *attrs[]);
int tipc_bclink_reset_stats(struct net *net);
+u32 tipc_bcast_get_broadcast_mode(struct net *net);
+u32 tipc_bcast_get_broadcast_ratio(struct net *net);
+
static inline void tipc_bcast_lock(struct net *net)
{
spin_lock_bh(&tipc_net(net)->bclock);
diff --git a/net/tipc/link.c b/net/tipc/link.c
index 341ecd796aa4..52d23b3ffaf5 100644
--- a/net/tipc/link.c
+++ b/net/tipc/link.c
@@ -2197,6 +2197,8 @@ int tipc_nl_add_bc_link(struct net *net, struct tipc_nl_msg *msg)
struct nlattr *attrs;
struct nlattr *prop;
struct tipc_net *tn = net_generic(net, tipc_net_id);
+ u32 bc_mode = tipc_bcast_get_broadcast_mode(net);
+ u32 bc_ratio = tipc_bcast_get_broadcast_ratio(net);
struct tipc_link *bcl = tn->bcl;
if (!bcl)
@@ -2233,6 +2235,12 @@ int tipc_nl_add_bc_link(struct net *net, struct tipc_nl_msg *msg)
goto attr_msg_full;
if (nla_put_u32(msg->skb, TIPC_NLA_PROP_WIN, bcl->window))
goto prop_msg_full;
+ if (nla_put_u32(msg->skb, TIPC_NLA_PROP_BROADCAST, bc_mode))
+ goto prop_msg_full;
+ if (bc_mode & BCLINK_MODE_SEL)
+ if (nla_put_u32(msg->skb, TIPC_NLA_PROP_BROADCAST_RATIO,
+ bc_ratio))
+ goto prop_msg_full;
nla_nest_end(msg->skb, prop);
err = __tipc_nl_add_bc_link_stat(msg->skb, &bcl->stats);
diff --git a/net/tipc/netlink.c b/net/tipc/netlink.c
index 99ee419210ba..5240f64e8ccc 100644
--- a/net/tipc/netlink.c
+++ b/net/tipc/netlink.c
@@ -110,7 +110,9 @@ const struct nla_policy tipc_nl_prop_policy[TIPC_NLA_PROP_MAX + 1] = {
[TIPC_NLA_PROP_UNSPEC] = { .type = NLA_UNSPEC },
[TIPC_NLA_PROP_PRIO] = { .type = NLA_U32 },
[TIPC_NLA_PROP_TOL] = { .type = NLA_U32 },
- [TIPC_NLA_PROP_WIN] = { .type = NLA_U32 }
+ [TIPC_NLA_PROP_WIN] = { .type = NLA_U32 },
+ [TIPC_NLA_PROP_BROADCAST] = { .type = NLA_U32 },
+ [TIPC_NLA_PROP_BROADCAST_RATIO] = { .type = NLA_U32 }
};
const struct nla_policy tipc_nl_bearer_policy[TIPC_NLA_BEARER_MAX + 1] = {
--
2.17.1
^ permalink raw reply related
* [net-next 3/3] tipc: smooth change between replicast and broadcast
From: Hoang Le @ 2019-02-19 11:30 UTC (permalink / raw)
To: jon.maloy, maloy, ying.xue, netdev, tipc-discussion
In-Reply-To: <20190219113054.13517-1-hoang.h.le@dektech.com.au>
Currently, a multicast stream may start out using replicast, because
there are few destinations, and then it should ideally switch to
L2/broadcast IGMP/multicast when the number of destinations grows beyond
a certain limit. The opposite should happen when the number decreases
below the limit.
To eliminate the risk of message reordering caused by method change,
a sending socket must stick to a previously selected method until it
enters an idle period of 5 seconds. Means there is a 5 seconds pause
in the traffic from the sender socket.
If the sender never makes such a pause, the method will never change,
and transmission may become very inefficient as the cluster grows.
With this commit, we allow such a switch between replicast and
broadcast without any need for a traffic pause.
Solution is to send a dummy message with only the header, also with
the SYN bit set, via broadcast or replicast. For the data message,
the SYN bit is set and sending via replicast or broadcast (inverse
method with dummy).
Then, at receiving side any messages follow first SYN bit message
(data or dummy message), they will be held in deferred queue until
another pair (dummy or data message) arrived in other link.
Acked-by: Jon Maloy <jon.maloy@ericsson.com>
Signed-off-by: Hoang Le <hoang.h.le@dektech.com.au>
---
net/tipc/bcast.c | 165 +++++++++++++++++++++++++++++++++++++++++++++-
net/tipc/bcast.h | 5 ++
net/tipc/msg.h | 10 +++
net/tipc/socket.c | 5 ++
4 files changed, 184 insertions(+), 1 deletion(-)
diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c
index 12b59268bdd6..de11a036821e 100644
--- a/net/tipc/bcast.c
+++ b/net/tipc/bcast.c
@@ -220,9 +220,24 @@ static void tipc_bcast_select_xmit_method(struct net *net, int dests,
}
/* Can current method be changed ? */
method->expires = jiffies + TIPC_METHOD_EXPIRE;
- if (method->mandatory || time_before(jiffies, exp))
+ if (method->mandatory)
return;
+ if (!(tipc_net(net)->capabilities & TIPC_MCAST_RBCTL) &&
+ time_before(jiffies, exp))
+ return;
+
+ /* Configuration as force 'broadcast' method */
+ if (bb->force_bcast) {
+ method->rcast = false;
+ return;
+ }
+ /* Configuration as force 'replicast' method */
+ if (bb->force_rcast) {
+ method->rcast = true;
+ return;
+ }
+ /* Configuration as 'autoselect' or default method */
/* Determine method to use now */
method->rcast = dests <= bb->bc_threshold;
}
@@ -285,6 +300,63 @@ static int tipc_rcast_xmit(struct net *net, struct sk_buff_head *pkts,
return 0;
}
+/* tipc_mcast_send_sync - deliver a dummy message with SYN bit
+ * @net: the applicable net namespace
+ * @skb: socket buffer to copy
+ * @method: send method to be used
+ * @dests: destination nodes for message.
+ * @cong_link_cnt: returns number of encountered congested destination links
+ * Returns 0 if success, otherwise errno
+ */
+static int tipc_mcast_send_sync(struct net *net, struct sk_buff *skb,
+ struct tipc_mc_method *method,
+ struct tipc_nlist *dests,
+ u16 *cong_link_cnt)
+{
+ struct sk_buff_head tmpq;
+ struct sk_buff *_skb;
+ struct tipc_msg *hdr, *_hdr;
+
+ /* Is a cluster supporting with new capabilities ? */
+ if (!(tipc_net(net)->capabilities & TIPC_MCAST_RBCTL))
+ return 0;
+
+ hdr = buf_msg(skb);
+ if (msg_user(hdr) == MSG_FRAGMENTER)
+ hdr = msg_get_wrapped(hdr);
+ if (msg_type(hdr) != TIPC_MCAST_MSG)
+ return 0;
+
+ /* Allocate dummy message */
+ _skb = tipc_buf_acquire(MCAST_H_SIZE, GFP_KERNEL);
+ if (!skb)
+ return -ENOMEM;
+
+ /* Preparing for 'synching' header */
+ msg_set_syn(hdr, 1);
+
+ /* Copy skb's header into a dummy header */
+ skb_copy_to_linear_data(_skb, hdr, MCAST_H_SIZE);
+ skb_orphan(_skb);
+
+ /* Reverse method for dummy message */
+ _hdr = buf_msg(_skb);
+ msg_set_size(_hdr, MCAST_H_SIZE);
+ msg_set_is_rcast(_hdr, !msg_is_rcast(hdr));
+
+ skb_queue_head_init(&tmpq);
+ __skb_queue_tail(&tmpq, _skb);
+ if (method->rcast)
+ tipc_bcast_xmit(net, &tmpq, cong_link_cnt);
+ else
+ tipc_rcast_xmit(net, &tmpq, dests, cong_link_cnt);
+
+ /* This queue should normally be empty by now */
+ __skb_queue_purge(&tmpq);
+
+ return 0;
+}
+
/* tipc_mcast_xmit - deliver message to indicated destination nodes
* and to identified node local sockets
* @net: the applicable net namespace
@@ -300,6 +372,9 @@ int tipc_mcast_xmit(struct net *net, struct sk_buff_head *pkts,
u16 *cong_link_cnt)
{
struct sk_buff_head inputq, localq;
+ struct sk_buff *skb;
+ struct tipc_msg *hdr;
+ bool rcast = method->rcast;
int rc = 0;
skb_queue_head_init(&inputq);
@@ -313,6 +388,18 @@ int tipc_mcast_xmit(struct net *net, struct sk_buff_head *pkts,
/* Send according to determined transmit method */
if (dests->remote) {
tipc_bcast_select_xmit_method(net, dests->remote, method);
+
+ skb = skb_peek(pkts);
+ hdr = buf_msg(skb);
+ if (msg_user(hdr) == MSG_FRAGMENTER)
+ hdr = msg_get_wrapped(hdr);
+ msg_set_is_rcast(hdr, method->rcast);
+
+ /* Switch method ? */
+ if (rcast != method->rcast)
+ tipc_mcast_send_sync(net, skb, method,
+ dests, cong_link_cnt);
+
if (method->rcast)
rc = tipc_rcast_xmit(net, pkts, dests, cong_link_cnt);
else
@@ -672,3 +759,79 @@ u32 tipc_bcast_get_broadcast_ratio(struct net *net)
return bb->rc_ratio;
}
+
+void tipc_mcast_filter_msg(struct sk_buff_head *defq,
+ struct sk_buff_head *inputq)
+{
+ struct sk_buff *skb, *_skb, *tmp;
+ struct tipc_msg *hdr, *_hdr;
+ bool match = false;
+ u32 node, port;
+
+ skb = skb_peek(inputq);
+ hdr = buf_msg(skb);
+
+ if (likely(!msg_is_syn(hdr) && skb_queue_empty(defq)))
+ return;
+
+ node = msg_orignode(hdr);
+ port = msg_origport(hdr);
+
+ /* Has the twin SYN message already arrived ? */
+ skb_queue_walk(defq, _skb) {
+ _hdr = buf_msg(_skb);
+ if (msg_orignode(_hdr) != node)
+ continue;
+ if (msg_origport(_hdr) != port)
+ continue;
+ match = true;
+ break;
+ }
+
+ if (!match) {
+ if (!msg_is_syn(hdr))
+ return;
+ __skb_dequeue(inputq);
+ __skb_queue_tail(defq, skb);
+ return;
+ }
+
+ /* Deliver non-SYN message from other link, otherwise queue it */
+ if (!msg_is_syn(hdr)) {
+ if (msg_is_rcast(hdr) != msg_is_rcast(_hdr))
+ return;
+ __skb_dequeue(inputq);
+ __skb_queue_tail(defq, skb);
+ return;
+ }
+
+ /* Queue non-SYN/SYN message from same link */
+ if (msg_is_rcast(hdr) == msg_is_rcast(_hdr)) {
+ __skb_dequeue(inputq);
+ __skb_queue_tail(defq, skb);
+ return;
+ }
+
+ /* Matching SYN messages => return the one with data, if any */
+ __skb_unlink(_skb, defq);
+ if (msg_data_sz(hdr)) {
+ kfree_skb(_skb);
+ } else {
+ __skb_dequeue(inputq);
+ kfree_skb(skb);
+ __skb_queue_tail(inputq, _skb);
+ }
+
+ /* Deliver subsequent non-SYN messages from same peer */
+ skb_queue_walk_safe(defq, _skb, tmp) {
+ _hdr = buf_msg(_skb);
+ if (msg_orignode(_hdr) != node)
+ continue;
+ if (msg_origport(_hdr) != port)
+ continue;
+ if (msg_is_syn(_hdr))
+ break;
+ __skb_unlink(_skb, defq);
+ __skb_queue_tail(inputq, _skb);
+ }
+}
diff --git a/net/tipc/bcast.h b/net/tipc/bcast.h
index 37c55e7347a5..484bde289d3a 100644
--- a/net/tipc/bcast.h
+++ b/net/tipc/bcast.h
@@ -67,11 +67,13 @@ void tipc_nlist_del(struct tipc_nlist *nl, u32 node);
/* Cookie to be used between socket and broadcast layer
* @rcast: replicast (instead of broadcast) was used at previous xmit
* @mandatory: broadcast/replicast indication was set by user
+ * @deferredq: defer queue to make message in order
* @expires: re-evaluate non-mandatory transmit method if we are past this
*/
struct tipc_mc_method {
bool rcast;
bool mandatory;
+ struct sk_buff_head deferredq;
unsigned long expires;
};
@@ -99,6 +101,9 @@ int tipc_bclink_reset_stats(struct net *net);
u32 tipc_bcast_get_broadcast_mode(struct net *net);
u32 tipc_bcast_get_broadcast_ratio(struct net *net);
+void tipc_mcast_filter_msg(struct sk_buff_head *defq,
+ struct sk_buff_head *inputq);
+
static inline void tipc_bcast_lock(struct net *net)
{
spin_lock_bh(&tipc_net(net)->bclock);
diff --git a/net/tipc/msg.h b/net/tipc/msg.h
index d7e4b8b93f9d..528ba9241acc 100644
--- a/net/tipc/msg.h
+++ b/net/tipc/msg.h
@@ -257,6 +257,16 @@ static inline void msg_set_src_droppable(struct tipc_msg *m, u32 d)
msg_set_bits(m, 0, 18, 1, d);
}
+static inline bool msg_is_rcast(struct tipc_msg *m)
+{
+ return msg_bits(m, 0, 18, 0x1);
+}
+
+static inline void msg_set_is_rcast(struct tipc_msg *m, bool d)
+{
+ msg_set_bits(m, 0, 18, 0x1, d);
+}
+
static inline void msg_set_size(struct tipc_msg *m, u32 sz)
{
m->hdr[0] = htonl((msg_word(m, 0) & ~0x1ffff) | sz);
diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index 8fc5acd4820d..de83eb1e718e 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -483,6 +483,7 @@ static int tipc_sk_create(struct net *net, struct socket *sock,
tsk_set_unreturnable(tsk, true);
if (sock->type == SOCK_DGRAM)
tsk_set_unreliable(tsk, true);
+ __skb_queue_head_init(&tsk->mc_method.deferredq);
}
trace_tipc_sk_create(sk, NULL, TIPC_DUMP_NONE, " ");
@@ -580,6 +581,7 @@ static int tipc_release(struct socket *sock)
sk->sk_shutdown = SHUTDOWN_MASK;
tipc_sk_leave(tsk);
tipc_sk_withdraw(tsk, 0, NULL);
+ __skb_queue_purge(&tsk->mc_method.deferredq);
sk_stop_timer(sk, &sk->sk_timer);
tipc_sk_remove(tsk);
@@ -2157,6 +2159,9 @@ static void tipc_sk_filter_rcv(struct sock *sk, struct sk_buff *skb,
if (unlikely(grp))
tipc_group_filter_msg(grp, &inputq, xmitq);
+ if (msg_type(hdr) == TIPC_MCAST_MSG)
+ tipc_mcast_filter_msg(&tsk->mc_method.deferredq, &inputq);
+
/* Validate and add to receive buffer if there is space */
while ((skb = __skb_dequeue(&inputq))) {
hdr = buf_msg(skb);
--
2.17.1
^ permalink raw reply related
* [iproute2-next 1/2] tipc: add link broadcast set method and ratio
From: Hoang Le @ 2019-02-19 11:36 UTC (permalink / raw)
To: jon.maloy, maloy, ying.xue, netdev, tipc-discussion
The command added here makes it possible to forcibly configure the
broadcast link to use either broadcast or replicast, in addition to
the already existing auto selection algorithm.
A sample usage is shown below:
$tipc link set broadcast BROADCAST
$tipc link set broadcast AUTOSELECT ratio 25
$tipc link set broadcast -h
Usage: tipc link set broadcast PROPERTY
PROPERTIES
BROADCAST - Forces all multicast traffic to be
transmitted via broadcast only,
irrespective of cluster size and number
of destinations
REPLICAST - Forces all multicast traffic to be
transmitted via replicast only,
irrespective of cluster size and number
of destinations
AUTOSELECT - Auto switching to broadcast or replicast
depending on cluster size and destination
node number
ratio SIZE - Set the AUTOSELECT criteria, percentage of
destination nodes vs cluster size
Acked-by: Jon Maloy <jon.maloy@ericsson.com>
Signed-off-by: Hoang Le <hoang.h.le@dektech.com.au>
---
include/uapi/linux/tipc_netlink.h | 2 +
tipc/link.c | 96 ++++++++++++++++++++++++++++++-
2 files changed, 97 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/tipc_netlink.h b/include/uapi/linux/tipc_netlink.h
index 0ebe02ef1a86..efb958fd167d 100644
--- a/include/uapi/linux/tipc_netlink.h
+++ b/include/uapi/linux/tipc_netlink.h
@@ -281,6 +281,8 @@ enum {
TIPC_NLA_PROP_TOL, /* u32 */
TIPC_NLA_PROP_WIN, /* u32 */
TIPC_NLA_PROP_MTU, /* u32 */
+ TIPC_NLA_PROP_BROADCAST, /* u32 */
+ TIPC_NLA_PROP_BROADCAST_RATIO, /* u32 */
__TIPC_NLA_PROP_MAX,
TIPC_NLA_PROP_MAX = __TIPC_NLA_PROP_MAX - 1
diff --git a/tipc/link.c b/tipc/link.c
index 43e26da3fa6b..e3b10bb7b3d4 100644
--- a/tipc/link.c
+++ b/tipc/link.c
@@ -28,6 +28,9 @@
#define PRIORITY_STR "priority"
#define TOLERANCE_STR "tolerance"
#define WINDOW_STR "window"
+#define BROADCAST_STR "broadcast"
+
+static const char tipc_bclink_name[] = "broadcast-link";
static int link_list_cb(const struct nlmsghdr *nlh, void *data)
{
@@ -521,7 +524,8 @@ static void cmd_link_set_help(struct cmdl *cmdl)
"PROPERTIES\n"
" tolerance TOLERANCE - Set link tolerance\n"
" priority PRIORITY - Set link priority\n"
- " window WINDOW - Set link window\n",
+ " window WINDOW - Set link window\n"
+ " broadcast BROADCAST - Set link broadcast\n",
cmdl->argv[0]);
}
@@ -585,6 +589,95 @@ static int cmd_link_set_prop(struct nlmsghdr *nlh, const struct cmd *cmd,
return msg_doit(nlh, link_get_cb, &prop);
}
+static void cmd_link_set_bcast_help(struct cmdl *cmdl)
+{
+ fprintf(stderr, "Usage: %s link set broadcast PROPERTY\n\n"
+ "PROPERTIES\n"
+ " BROADCAST - Forces all multicast traffic to be\n"
+ " transmitted via broadcast only,\n"
+ " irrespective of cluster size and number\n"
+ " of destinations\n\n"
+ " REPLICAST - Forces all multicast traffic to be\n"
+ " transmitted via replicast only,\n"
+ " irrespective of cluster size and number\n"
+ " of destinations\n\n"
+ " AUTOSELECT - Auto switching to broadcast or replicast\n"
+ " depending on cluster size and destination\n"
+ " node number\n\n"
+ " ratio SIZE - Set the AUTOSELECT criteria, percentage of\n"
+ " destination nodes vs cluster size\n\n",
+ cmdl->argv[0]);
+}
+
+static int cmd_link_set_bcast(struct nlmsghdr *nlh, const struct cmd *cmd,
+ struct cmdl *cmdl, void *data)
+{
+ char buf[MNL_SOCKET_BUFFER_SIZE];
+ struct nlattr *props;
+ struct nlattr *attrs;
+ struct opt *opt;
+ struct opt opts[] = {
+ { "BROADCAST", OPT_KEY, NULL },
+ { "REPLICAST", OPT_KEY, NULL },
+ { "AUTOSELECT", OPT_KEY, NULL },
+ { "ratio", OPT_KEYVAL, NULL },
+ { NULL }
+ };
+ int method = 0;
+
+ if (help_flag) {
+ (cmd->help)(cmdl);
+ return -EINVAL;
+ }
+
+ if (parse_opts(opts, cmdl) < 0)
+ return -EINVAL;
+
+ for (opt = opts; opt->key; opt++)
+ if (opt->val)
+ break;
+
+ if (!opt || !opt->key) {
+ (cmd->help)(cmdl);
+ return -EINVAL;
+ }
+
+ nlh = msg_init(buf, TIPC_NL_LINK_SET);
+ if (!nlh) {
+ fprintf(stderr, "error, message initialisation failed\n");
+ return -1;
+ }
+
+ attrs = mnl_attr_nest_start(nlh, TIPC_NLA_LINK);
+ /* Direct to broadcast-link setting */
+ mnl_attr_put_strz(nlh, TIPC_NLA_LINK_NAME, tipc_bclink_name);
+ props = mnl_attr_nest_start(nlh, TIPC_NLA_LINK_PROP);
+
+ if (get_opt(opts, "BROADCAST"))
+ method = 0x1;
+ else if (get_opt(opts, "REPLICAST"))
+ method = 0x2;
+ else if (get_opt(opts, "AUTOSELECT"))
+ method = 0x4;
+
+ opt = get_opt(opts, "ratio");
+ if (!method && !opt) {
+ (cmd->help)(cmdl);
+ return -EINVAL;
+ }
+
+ if (method)
+ mnl_attr_put_u32(nlh, TIPC_NLA_PROP_BROADCAST, method);
+
+ if (opt)
+ mnl_attr_put_u32(nlh, TIPC_NLA_PROP_BROADCAST_RATIO,
+ atoi(opt->val));
+
+ mnl_attr_nest_end(nlh, props);
+ mnl_attr_nest_end(nlh, attrs);
+ return msg_doit(nlh, NULL, NULL);
+}
+
static int cmd_link_set(struct nlmsghdr *nlh, const struct cmd *cmd,
struct cmdl *cmdl, void *data)
{
@@ -592,6 +685,7 @@ static int cmd_link_set(struct nlmsghdr *nlh, const struct cmd *cmd,
{ PRIORITY_STR, cmd_link_set_prop, cmd_link_set_help },
{ TOLERANCE_STR, cmd_link_set_prop, cmd_link_set_help },
{ WINDOW_STR, cmd_link_set_prop, cmd_link_set_help },
+ { BROADCAST_STR, cmd_link_set_bcast, cmd_link_set_bcast_help },
{ NULL }
};
--
2.17.1
^ permalink raw reply related
* [iproute2-next 2/2] tipc: add link broadcast get
From: Hoang Le @ 2019-02-19 11:36 UTC (permalink / raw)
To: jon.maloy, maloy, ying.xue, netdev, tipc-discussion
In-Reply-To: <20190219113610.13672-1-hoang.h.le@dektech.com.au>
The command prints the actually method that multicast
is running in the system.
Also 'ratio' value for AUTOSELECT method.
A sample usage is shown below:
$tipc link get broadcast
BROADCAST
$tipc link get broadcast
AUTOSELECT ratio:30%
$tipc link get broadcast -j -p
[ {
"method": "AUTOSELECT"
},{
"ratio": 30
} ]
Acked-by: Jon Maloy <jon.maloy@ericsson.com>
Signed-off-by: Hoang Le <hoang.h.le@dektech.com.au>
---
tipc/link.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 84 insertions(+), 1 deletion(-)
diff --git a/tipc/link.c b/tipc/link.c
index e3b10bb7b3d4..e123c1863575 100644
--- a/tipc/link.c
+++ b/tipc/link.c
@@ -175,10 +175,92 @@ static void cmd_link_get_help(struct cmdl *cmdl)
"PROPERTIES\n"
" tolerance - Get link tolerance\n"
" priority - Get link priority\n"
- " window - Get link window\n",
+ " window - Get link window\n"
+ " broadcast - Get link broadcast\n",
cmdl->argv[0]);
}
+static int cmd_link_get_bcast_cb(const struct nlmsghdr *nlh, void *data)
+{
+ int *prop = data;
+ int prop_ratio = TIPC_NLA_PROP_BROADCAST_RATIO;
+ struct genlmsghdr *genl = mnl_nlmsg_get_payload(nlh);
+ struct nlattr *info[TIPC_NLA_MAX + 1] = {};
+ struct nlattr *attrs[TIPC_NLA_LINK_MAX + 1] = {};
+ struct nlattr *props[TIPC_NLA_PROP_MAX + 1] = {};
+ int bc_mode;
+
+ mnl_attr_parse(nlh, sizeof(*genl), parse_attrs, info);
+ if (!info[TIPC_NLA_LINK])
+ return MNL_CB_ERROR;
+
+ mnl_attr_parse_nested(info[TIPC_NLA_LINK], parse_attrs, attrs);
+ if (!attrs[TIPC_NLA_LINK_PROP])
+ return MNL_CB_ERROR;
+
+ mnl_attr_parse_nested(attrs[TIPC_NLA_LINK_PROP], parse_attrs, props);
+ if (!props[*prop])
+ return MNL_CB_ERROR;
+
+ bc_mode = mnl_attr_get_u32(props[*prop]);
+
+ new_json_obj(json);
+ open_json_object(NULL);
+ switch (bc_mode) {
+ case 0x1:
+ print_string(PRINT_ANY, "method", "%s\n", "BROADCAST");
+ break;
+ case 0x2:
+ print_string(PRINT_ANY, "method", "%s\n", "REPLICAST");
+ break;
+ case 0x4:
+ print_string(PRINT_ANY, "method", "%s", "AUTOSELECT");
+ close_json_object();
+ open_json_object(NULL);
+ print_uint(PRINT_ANY, "ratio", " ratio:%u%\n",
+ mnl_attr_get_u32(props[prop_ratio]));
+ break;
+ default:
+ print_string(PRINT_ANY, NULL, "UNKNOWN\n", NULL);
+ break;
+ }
+ close_json_object();
+ delete_json_obj();
+ return MNL_CB_OK;
+}
+
+static void cmd_link_get_bcast_help(struct cmdl *cmdl)
+{
+ fprintf(stderr, "Usage: %s link get PPROPERTY\n\n"
+ "PROPERTIES\n"
+ " broadcast - Get link broadcast\n",
+ cmdl->argv[0]);
+}
+
+static int cmd_link_get_bcast(struct nlmsghdr *nlh, const struct cmd *cmd,
+ struct cmdl *cmdl, void *data)
+{
+ int prop = TIPC_NLA_PROP_BROADCAST;
+ char buf[MNL_SOCKET_BUFFER_SIZE];
+ struct nlattr *attrs;
+
+ if (help_flag) {
+ (cmd->help)(cmdl);
+ return -EINVAL;
+ }
+
+ nlh = msg_init(buf, TIPC_NL_LINK_GET);
+ if (!nlh) {
+ fprintf(stderr, "error, message initialisation failed\n");
+ return -1;
+ }
+ attrs = mnl_attr_nest_start(nlh, TIPC_NLA_LINK);
+ /* Direct to broadcast-link setting */
+ mnl_attr_put_strz(nlh, TIPC_NLA_LINK_NAME, tipc_bclink_name);
+ mnl_attr_nest_end(nlh, attrs);
+ return msg_doit(nlh, cmd_link_get_bcast_cb, &prop);
+}
+
static int cmd_link_get(struct nlmsghdr *nlh, const struct cmd *cmd,
struct cmdl *cmdl, void *data)
{
@@ -186,6 +268,7 @@ static int cmd_link_get(struct nlmsghdr *nlh, const struct cmd *cmd,
{ PRIORITY_STR, cmd_link_get_prop, cmd_link_get_help },
{ TOLERANCE_STR, cmd_link_get_prop, cmd_link_get_help },
{ WINDOW_STR, cmd_link_get_prop, cmd_link_get_help },
+ { BROADCAST_STR, cmd_link_get_bcast, cmd_link_get_bcast_help },
{ NULL }
};
--
2.17.1
^ permalink raw reply related
* Re: [RFC PATCH net-next v3 12/21] ethtool: provide permanent hardware address in GET_INFO request
From: Michal Kubecek @ 2019-02-19 11:36 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, David Miller, Andrew Lunn, Jakub Kicinski, linux-kernel
In-Reply-To: <20190219102400.GC3080@nanopsycho>
On Tue, Feb 19, 2019 at 11:24:00AM +0100, Jiri Pirko wrote:
> Mon, Feb 18, 2019 at 07:22:24PM CET, mkubecek@suse.cz wrote:
> >Add information about permanent hadware address of a device (as provided by
> >ETHTOOL_GPERMADDR ioctl command) in GET_INFO reply if ETH_INFO_IM_PERMADDR
> >flag is set in the request.
> >
> >There is no separate attribute for hardware address length as nla_len gives
> >this information. The reply also provides address type (net_device::type).
> >
> >Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
> >---
> > Documentation/networking/ethtool-netlink.txt | 9 ++++-
> > include/uapi/linux/ethtool_netlink.h | 12 +++++-
> > net/ethtool/info.c | 39 ++++++++++++++++++++
> > 3 files changed, 58 insertions(+), 2 deletions(-)
> >
> >diff --git a/Documentation/networking/ethtool-netlink.txt b/Documentation/networking/ethtool-netlink.txt
> >index b6999a2167e8..1e615e111262 100644
> >--- a/Documentation/networking/ethtool-netlink.txt
> >+++ b/Documentation/networking/ethtool-netlink.txt
> >@@ -239,6 +239,9 @@ Kernel response contents:
> > ETHA_DRVINFO_FWVERSION (string) firmware version
> > ETHA_DRVINFO_BUSINFO (string) device bus address
> > ETHA_DRVINFO_EROM_VER (string) expansion ROM version
> >+ ETHA_INFO_PERMADDR (nested)
> >+ ETHA_PERMADDR_ADDRESS (binary) permanent HW address
>
> I think this is a nice example of thing that should not be exposed with
> ethtool but rather via rtnetlink, alongside with the actual hw address.
>
> [...]
I guess you are right. As we don't have to query the driver and just
read the information from struct net_device, rtnetlink does indeed seem
more appropriate.
Michal
^ permalink raw reply
* Re: [PATCH net-next v3 1/2] net: phy: at803x: don't inline helpers
From: Marc Gonzalez @ 2019-02-19 11:44 UTC (permalink / raw)
To: Vinod Koul, David Miller, netdev
Cc: Linux ARM, Bjorn Andersson, Niklas Cassel, Andrew Lunn,
Florian Fainelli, Nori Sekhar, Peter Ujfalusi
In-Reply-To: <20190219061800.31025-2-vkoul@kernel.org>
On 19/02/2019 07:17, Vinod Koul wrote:
> Some helpers were inlined, but makes more sense to allow compiler
> to do the right optimizations instead, so remove inline for
> at803x_disable_rx_delay() and at803x_disable_tx_delay()
I would word it slightly differently:
Some helpers were declared with the "inline" function specifier.
It is preferable to let the compiler pick the right optimizations,
so drop the specifier.
[ This is just a random suggestion, feel free to ignore ]
> Reviewed-by: Niklas Cassel <niklas.cassel@linaro.org>
> Signed-off-by: Vinod Koul <vkoul@kernel.org>
> ---
> drivers/net/phy/at803x.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
> index 8ff12938ab47..c6e7d800fd7a 100644
> --- a/drivers/net/phy/at803x.c
> +++ b/drivers/net/phy/at803x.c
> @@ -110,13 +110,13 @@ static int at803x_debug_reg_mask(struct phy_device *phydev, u16 reg,
> return phy_write(phydev, AT803X_DEBUG_DATA, val);
> }
>
> -static inline int at803x_disable_rx_delay(struct phy_device *phydev)
> +static int at803x_disable_rx_delay(struct phy_device *phydev)
> {
> return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_0,
> AT803X_DEBUG_RX_CLK_DLY_EN, 0);
> }
>
> -static inline int at803x_disable_tx_delay(struct phy_device *phydev)
> +static int at803x_disable_tx_delay(struct phy_device *phydev)
> {
> return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_5,
> AT803X_DEBUG_TX_CLK_DLY_EN, 0);
Reviewed-by: Marc Gonzalez <marc.w.gonzalez@free.fr>
^ permalink raw reply
* Re: [PATCH bpf-next] bpf: add skb->queue_mapping write access from tc clsact
From: Daniel Borkmann @ 2019-02-19 11:46 UTC (permalink / raw)
To: Jesper Dangaard Brouer, netdev, Daniel Borkmann,
Alexei Starovoitov
In-Reply-To: <155057184028.20935.11848804617158437103.stgit@firesoul>
On 02/19/2019 11:24 AM, Jesper Dangaard Brouer wrote:
> The skb->queue_mapping already have read access, via __sk_buff->queue_mapping.
>
> This patch allow BPF tc qdisc clsact write access to the queue_mapping via
> tc_cls_act_is_valid_access.
>
> It is already possible to change this via TC filter action skbedit
> tc-skbedit(8). Due to the lack of TC examples, lets show one:
>
> # tc qdisc add dev ixgbe1 handle ffff: ingress
> # tc filter add dev ixgbe1 parent ffff: matchall action skbedit queue_mapping 5
> # tc filter list dev ixgbe1 parent ffff:
Using handles was in the old days, if we add examples, then lets do
something more user friendly ;)
# tc qdisc add dev ixgbe1 clsact
# tc filter replace dev ixgbe1 ingress matchall action skbedit queue_mapping 5
# tc filter list dev ixgbe1 ingress
> The most common mistake is that XPS (Transmit Packet Steering) takes
> precedence over setting skb->queue_mapping. XPS is configured per DEVICE
> via /sys/class/net/DEVICE/queues/tx-*/xps_cpus via a CPU hex mask. To
> disable set mask=00.
>
> The purpose of changing skb->queue_mapping is to influence the selection of
> the net_device "txq" (struct netdev_queue), which influence selection of
> the qdisc "root_lock" (via txq->qdisc->q.lock) and txq->_xmit_lock. When
> using the MQ qdisc the txq->qdisc points to different qdiscs and associated
> locks, and HARD_TX_LOCK (txq->_xmit_lock), allowing for CPU scalability.
>
> Due to lack of TC examples, lets show howto attach clsact BPF programs:
>
> # tc qdisc add dev ixgbe2 clsact
> # tc filter replace dev ixgbe2 egress bpf da obj XXX_kern.o sec tc_qmap2cpu
> # tc filter list dev ixgbe2 egress
>
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
> ---
> net/core/filter.c | 14 +++++++++++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 353735575204..d05ae8d05397 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -6238,6 +6238,7 @@ static bool tc_cls_act_is_valid_access(int off, int size,
> case bpf_ctx_range(struct __sk_buff, tc_classid):
> case bpf_ctx_range_till(struct __sk_buff, cb[0], cb[4]):
> case bpf_ctx_range(struct __sk_buff, tstamp):
> + case bpf_ctx_range(struct __sk_buff, queue_mapping):
> break;
> default:
> return false;
> @@ -6642,9 +6643,16 @@ static u32 bpf_convert_ctx_access(enum bpf_access_type type,
> break;
>
> case offsetof(struct __sk_buff, queue_mapping):
> - *insn++ = BPF_LDX_MEM(BPF_H, si->dst_reg, si->src_reg,
> - bpf_target_off(struct sk_buff, queue_mapping, 2,
> - target_size));
> + if (type == BPF_WRITE)
> + *insn++ = BPF_STX_MEM(BPF_H, si->dst_reg, si->src_reg,
> + bpf_target_off(struct sk_buff,
> + queue_mapping,
> + 2, target_size));
> + else
> + *insn++ = BPF_LDX_MEM(BPF_H, si->dst_reg, si->src_reg,
> + bpf_target_off(struct sk_buff,
> + queue_mapping,
> + 2, target_size));
One thing we should avoid would be to allow user to write NO_QUEUE_MAPPING
into skb->queue_mapping so we don't hit the warn in sk_tx_queue_set(), I'd
add this into the ctx rewrite here.
> break;
>
> case offsetof(struct __sk_buff, vlan_present):
>
^ permalink raw reply
* Re: [RFC PATCH net-next v3 00/21] ethtool netlink interface, part 1
From: Michal Kubecek @ 2019-02-19 11:57 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, David Miller, Andrew Lunn, Jakub Kicinski, linux-kernel
In-Reply-To: <20190219103508.GD3080@nanopsycho>
On Tue, Feb 19, 2019 at 11:35:08AM +0100, Jiri Pirko wrote:
> >- some features provided by ethtool would rather belong to devlink (and
> > some are already superseded by devlink); however, only few drivers
> > provide devlink interface at the moment and as recent discussion on
> > flashing revealed, we cannot rely on devlink's presence
>
> Could you explain why please?
What I mean is the problem discussed under Jakub's devlink flash
patchset: that he couldn't implement only the devlink callback in nfp
and rely on the generic fallback to devlink because it wouldn't work if
devlink is built as a module.
But I think this should be addressed. If we agree that flashing (and
other features provided by ethtool at the moment) rather belongs to
devlink (which nobody seems to oppose), we should rather try to make it
possible for drivers to provide only the devlink callback and gradually
move all in-tree drivers to doing so. (And one day, remove it from
ethtool_ops.) It doesn't seem to make much sense to have devlink as
a module in such scenario.
Michal
^ permalink raw reply
* Re: [PATCH net-next v3 2/2] net: phy: at803x: disable delay only for RGMII mode
From: Marc Gonzalez @ 2019-02-19 11:57 UTC (permalink / raw)
To: Vinod Koul, David S Miller, netdev
Cc: MSM, Bjorn Andersson, Niklas Cassel, Andrew Lunn,
Florian Fainelli, Nori Sekhar, Peter Ujfalusi
In-Reply-To: <20190219061800.31025-3-vkoul@kernel.org>
On 19/02/2019 07:18, Vinod Koul wrote:
> Per "Documentation/devicetree/bindings/net/ethernet.txt" RGMII mode
> should not have delay in PHY whereas RGMII_ID and RGMII_RXID/RGMII_TXID
> can have delay in phy.
PHY or phy? :-)
> So disable the delay only for RGMII mode and enable for other modes.
> Also treat the default case as disabled delays.
>
> Fixes: cd28d1d6e52e: ("net: phy: at803x: Disable phy delay for RGMII mode")
> Reported-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> Reviewed-by: Niklas Cassel <niklas.cassel@linaro.org>
> Tested-by: Peter Ujfalusi <peter.ujflausi@ti.com>
> Signed-off-by: Vinod Koul <vkoul@kernel.org>
> ---
> drivers/net/phy/at803x.c | 47 ++++++++++++++++++++++++++++++----------
> 1 file changed, 36 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
> index c6e7d800fd7a..dc1b13f7fc12 100644
> --- a/drivers/net/phy/at803x.c
> +++ b/drivers/net/phy/at803x.c
> @@ -110,6 +110,18 @@ static int at803x_debug_reg_mask(struct phy_device *phydev, u16 reg,
> return phy_write(phydev, AT803X_DEBUG_DATA, val);
> }
>
> +static int at803x_enable_rx_delay(struct phy_device *phydev)
> +{
> + return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_0, 0,
> + AT803X_DEBUG_RX_CLK_DLY_EN);
> +}
> +
> +static int at803x_enable_tx_delay(struct phy_device *phydev)
> +{
> + return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_5, 0,
> + AT803X_DEBUG_TX_CLK_DLY_EN);
> +}
> +
> static int at803x_disable_rx_delay(struct phy_device *phydev)
> {
> return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_0,
> @@ -255,23 +267,36 @@ static int at803x_config_init(struct phy_device *phydev)
> if (ret < 0)
> return ret;
>
> - if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID ||
> - phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
> - phydev->interface == PHY_INTERFACE_MODE_RGMII) {
> - ret = at803x_disable_rx_delay(phydev);
> + /* The hardware register default is RX and TX delay enabled, so lets
> + * first disable the RX and TX delays in phy and enable them based
> + * on the mode selected
> + */
"let's" (let us)
For the record, AFAIR, the default is not *quite* RX and TX enabled:
https://www.spinics.net/lists/netdev/msg444527.html
RX: enabled at HW reset
TX: disabled at HW reset, but retains value after SW reset
> + ret = at803x_disable_rx_delay(phydev);
> + if (ret < 0)
> + return ret;
> + ret = at803x_disable_tx_delay(phydev);
> + if (ret < 0)
> + return ret;
> +
> + if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
> + phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID) {
> + /* If RGMII_ID or RGMII_RXID are specified enable RX delay,
> + * otherwise keep it disabled
> + */
> + ret = at803x_enable_rx_delay(phydev);
> if (ret < 0)
> return ret;
> }
>
> - if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID ||
> - phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
> - phydev->interface == PHY_INTERFACE_MODE_RGMII) {
> - ret = at803x_disable_tx_delay(phydev);
> - if (ret < 0)
> - return ret;
> + if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
> + phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID) {
> + /* If RGMII_ID or RGMII_TXID are specified enable TX delay,
> + * otherwise keep it disabled
> + */
> + ret = at803x_enable_tx_delay(phydev);
> }
>
> - return 0;
> + return ret;
> }
IMO, the asymmetry in error handling for RX and TX is unfortunate.
Didn't you like the way it was done in my old patch? :-)
https://www.spinics.net/lists/netdev/msg445053.html
Regards.
^ permalink raw reply
* Re: [PATCH net-next v4 07/17] net: sched: protect filter_chain list with filter_chain_lock mutex
From: Vlad Buslov @ 2019-02-19 12:31 UTC (permalink / raw)
To: Cong Wang
Cc: Ido Schimmel, netdev@vger.kernel.org, jhs@mojatatu.com,
jiri@resnulli.us, davem@davemloft.net, ast@kernel.org,
daniel@iogearbox.net
In-Reply-To: <CAM_iQpWXF1Yq2Yic0J3KTxo+h5UQTc1vwEcR-cs1424dnxd9-g@mail.gmail.com>
On Tue 19 Feb 2019 at 05:26, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Fri, Feb 15, 2019 at 7:35 AM Vlad Buslov <vladbu@mellanox.com> wrote:
>>
>> Another problem that I found in cls_fw and cls_route is that they set
>> arg->stop when empty. Both of them have code unchanged since it was
>> committed initially in 2005 so I assume this convention is no longer
>> relevant because all other classifiers don't do that (they only set
>> arg->stop when arg->fn returns negative value).
>>
>
> The question is why do you want to use arg->stop==0 as
> an indication for emptiness? Isn't what arg->count==0
> supposed to be?
Good question! I initially wanted to implement it like that, but
reconsidered because iterating through all filters on classifier to
count them is O(N), and terminating on first filter and relying on
arg->stop==1 is constant time. Making function that is called
"tcf_proto_is_empty" linear on number of filters seemed sloppy to me...
^ permalink raw reply
* Re: [PATCH net-next v3 2/2] net: phy: at803x: disable delay only for RGMII mode
From: Niklas Cassel @ 2019-02-19 12:33 UTC (permalink / raw)
To: Marc Gonzalez
Cc: Vinod Koul, David S Miller, netdev, MSM, Bjorn Andersson,
Andrew Lunn, Florian Fainelli, Nori Sekhar, Peter Ujfalusi
In-Reply-To: <1dddc455-309d-5e7d-ca26-479fe800a013@free.fr>
On Tue, Feb 19, 2019 at 12:57:18PM +0100, Marc Gonzalez wrote:
> On 19/02/2019 07:18, Vinod Koul wrote:
>
> > Per "Documentation/devicetree/bindings/net/ethernet.txt" RGMII mode
> > should not have delay in PHY whereas RGMII_ID and RGMII_RXID/RGMII_TXID
> > can have delay in phy.
>
> PHY or phy? :-)
>
> > So disable the delay only for RGMII mode and enable for other modes.
> > Also treat the default case as disabled delays.
> >
> > Fixes: cd28d1d6e52e: ("net: phy: at803x: Disable phy delay for RGMII mode")
> > Reported-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> > Reviewed-by: Niklas Cassel <niklas.cassel@linaro.org>
> > Tested-by: Peter Ujfalusi <peter.ujflausi@ti.com>
> > Signed-off-by: Vinod Koul <vkoul@kernel.org>
> > ---
> > drivers/net/phy/at803x.c | 47 ++++++++++++++++++++++++++++++----------
> > 1 file changed, 36 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
> > index c6e7d800fd7a..dc1b13f7fc12 100644
> > --- a/drivers/net/phy/at803x.c
> > +++ b/drivers/net/phy/at803x.c
> > @@ -110,6 +110,18 @@ static int at803x_debug_reg_mask(struct phy_device *phydev, u16 reg,
> > return phy_write(phydev, AT803X_DEBUG_DATA, val);
> > }
> >
> > +static int at803x_enable_rx_delay(struct phy_device *phydev)
> > +{
> > + return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_0, 0,
> > + AT803X_DEBUG_RX_CLK_DLY_EN);
> > +}
> > +
> > +static int at803x_enable_tx_delay(struct phy_device *phydev)
> > +{
> > + return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_5, 0,
> > + AT803X_DEBUG_TX_CLK_DLY_EN);
> > +}
> > +
> > static int at803x_disable_rx_delay(struct phy_device *phydev)
> > {
> > return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_0,
> > @@ -255,23 +267,36 @@ static int at803x_config_init(struct phy_device *phydev)
> > if (ret < 0)
> > return ret;
> >
> > - if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID ||
> > - phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
> > - phydev->interface == PHY_INTERFACE_MODE_RGMII) {
> > - ret = at803x_disable_rx_delay(phydev);
> > + /* The hardware register default is RX and TX delay enabled, so lets
> > + * first disable the RX and TX delays in phy and enable them based
> > + * on the mode selected
> > + */
>
> "let's" (let us)
>
> For the record, AFAIR, the default is not *quite* RX and TX enabled:
>
> https://www.spinics.net/lists/netdev/msg444527.html
Hello Marc,
>
> RX: enabled at HW reset
> TX: disabled at HW reset, but retains value after SW reset
You are correct of course.
However, since the bootloader might have enabled delay on
TX, I still think that this patch does the right thing by
starting out with disabling delays for both RX and TX.
But we should probably make the comment more elaborate:
The value after HW reset is RX delay enabled and TX delay disabled.
The value after SW reset is RX delay enabled, while TX delay retains
the value before reset.
In order to not depend on reset values, start off by disabling both
delays.
Kind regards,
Niklas
>
> > + ret = at803x_disable_rx_delay(phydev);
> > + if (ret < 0)
> > + return ret;
> > + ret = at803x_disable_tx_delay(phydev);
> > + if (ret < 0)
> > + return ret;
> > +
> > + if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
> > + phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID) {
> > + /* If RGMII_ID or RGMII_RXID are specified enable RX delay,
> > + * otherwise keep it disabled
> > + */
> > + ret = at803x_enable_rx_delay(phydev);
> > if (ret < 0)
> > return ret;
> > }
> >
> > - if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID ||
> > - phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
> > - phydev->interface == PHY_INTERFACE_MODE_RGMII) {
> > - ret = at803x_disable_tx_delay(phydev);
> > - if (ret < 0)
> > - return ret;
> > + if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
> > + phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID) {
> > + /* If RGMII_ID or RGMII_TXID are specified enable TX delay,
> > + * otherwise keep it disabled
> > + */
> > + ret = at803x_enable_tx_delay(phydev);
> > }
> >
> > - return 0;
> > + return ret;
> > }
>
> IMO, the asymmetry in error handling for RX and TX is unfortunate.
>
> Didn't you like the way it was done in my old patch? :-)
>
> https://www.spinics.net/lists/netdev/msg445053.html
>
> Regards.
^ permalink raw reply
* Re: [RFC PATCH net-next v3 00/21] ethtool netlink interface, part 1
From: Jiri Pirko @ 2019-02-19 12:27 UTC (permalink / raw)
To: Michal Kubecek
Cc: netdev, David Miller, Andrew Lunn, Jakub Kicinski, linux-kernel
In-Reply-To: <20190219115727.GE23151@unicorn.suse.cz>
Tue, Feb 19, 2019 at 12:57:27PM CET, mkubecek@suse.cz wrote:
>On Tue, Feb 19, 2019 at 11:35:08AM +0100, Jiri Pirko wrote:
>> >- some features provided by ethtool would rather belong to devlink (and
>> > some are already superseded by devlink); however, only few drivers
>> > provide devlink interface at the moment and as recent discussion on
>> > flashing revealed, we cannot rely on devlink's presence
>>
>> Could you explain why please?
>
>What I mean is the problem discussed under Jakub's devlink flash
>patchset: that he couldn't implement only the devlink callback in nfp
>and rely on the generic fallback to devlink because it wouldn't work if
>devlink is built as a module.
So let's fix that.
>
>But I think this should be addressed. If we agree that flashing (and
>other features provided by ethtool at the moment) rather belongs to
>devlink (which nobody seems to oppose), we should rather try to make it
>possible for drivers to provide only the devlink callback and gradually
>move all in-tree drivers to doing so. (And one day, remove it from
>ethtool_ops.) It doesn't seem to make much sense to have devlink as
>a module in such scenario.
Agreed.
>
>Michal
^ permalink raw reply
* Re: [PATCH net-next v3 2/2] net: phy: at803x: disable delay only for RGMII mode
From: Marc Gonzalez @ 2019-02-19 12:41 UTC (permalink / raw)
To: Niklas Cassel
Cc: Vinod Koul, David Miller, netdev, MSM, Bjorn Andersson,
Andrew Lunn, Florian Fainelli, Nori Sekhar, Peter Ujfalusi
In-Reply-To: <20190219123339.GA13172@centauri.lan>
On 19/02/2019 13:33, Niklas Cassel wrote:
> However, since the bootloader might have enabled delay on
> TX, I still think that this patch does the right thing by
> starting out with disabling delays for both RX and TX.
>
> But we should probably make the comment more elaborate:
>
> The value after HW reset is RX delay enabled and TX delay disabled.
> The value after SW reset is RX delay enabled, while TX delay retains
> the value before reset.
> In order to not depend on reset values, start off by disabling both
> delays.
Ultimately, the two patches do the same thing, AFAICT ;-)
I was just arguing that my way was better because... errr... because
it was my way! :-)
Regards.
^ permalink raw reply
* Re: [PATCH mlx5-next v1] net/mlx5: Factor out HCA capabilities functions
From: Leon Romanovsky @ 2019-02-19 12:42 UTC (permalink / raw)
To: Doug Ledford, Jason Gunthorpe
Cc: RDMA mailing list, Saeed Mahameed, linux-netdev
In-Reply-To: <20190217111102.13608-1-leon@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 650 bytes --]
On Sun, Feb 17, 2019 at 01:11:02PM +0200, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@mellanox.com>
>
> Combine all HCA capabilities setters under one function
> and compile out the ODP related function in case kernel
> was compiled without ODP support.
>
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> ---
> Changelog v0->v1:
> * Embedded config option check into ODP capability function flow
> ---
> .../net/ethernet/mellanox/mlx5/core/main.c | 46 +++++++++++++------
> 1 file changed, 31 insertions(+), 15 deletions(-)
>
Applied to mlx5-next branch,
37b6bb77c6fd net/mlx5: Factor out HCA capabilities functions
Thanks
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: L2TPv3 offset
From: James Chapman @ 2019-02-19 12:40 UTC (permalink / raw)
To: t.martitz; +Cc: davem, netdev
In-Reply-To: <OF967393CB.A0BF4494-ONC12583A6.00323DFC-C12583A6.00330730@avm.de>
On 19/02/2019 09:17, t.martitz@avm.de wrote:
>
> Hello,
>
> I saw that you removed the offset option from l2tp sessions in Linux
> 4.16 (commit 900631ee6a2651dc4fbaecb8ef9fa5f1e3378853 l2tp: remove
> configurable payload offset). Since we need something like that I'm
> reaching out to you.
>
Adding netdev.
>
>
> Our use case is pseudo-wire over l2tp, so we encapsulate ethernet
> within l2tpv3. This has lots of benifits for us, but one remarkable
> drawback is that the receiver, who's removing the encapsulation, will
> see a misaligned inner IP header.
>
> So we planned to use the offset to correct that. But I saw that the
> offset was removed not too long ago. Now I see that there is still
> "l2specific_len" / L2TP_ATTR_L2SPEC_LEN that we can use for pretty
> much the same purpose but I get the feeling that this should only be 0
> or 4 while we would need 2 or 6. I get this feeling because "ip l2tp"
> of the iproute2 package does not allow setting this at all and
> hardcodes 0 or 4 based on the type.
>
In L2TPv3, any payload offset would need to be implemented by defining a
new L2SpecificSublayer type since the L2TPv3 header has no offset field.
There are two standard L2TPv3 sublayers supported - None and Default.
These are fixed size (0 and 4 bytes respectively), hence the kernel now
ignores L2TP_ATTR_L2SPEC_LEN since the length can be derived from
L2TP_ATTR_L2SPEC_TYPE. For reference, the set of defined types are
listed at
https://www.iana.org/assignments/l2tp-parameters/l2tp-parameters.xhtml#l2tp-parameters-37.
No L2SpecificSublayer type was ever defined for L2TPv3 to allow a
configurable payload offset in ethernet pseudowires.
>
> So my question is what should we do know? Being based on removed
> functionality is kind of bad, but we must fix the misaligned inner ip
> header.
> We control the sender and receiver so we could apply all kinds of
> hacks, but we would rather find a solution compliant with the Internet
> community.
>
If you really need to insert padding in transmitted L2TPv3 packets
between the L2TPv3 header and its payload, one option is to define a new
L2SpecificHeaderType and patch the kernel to accept it.
If you control both the sender and receiver, is using FOU an option?
James
^ permalink raw reply
* Re: [PATCH v3] net: ns83820: code cleanup for ns83820_probe_phy()
From: Walter Harms @ 2019-02-19 12:42 UTC (permalink / raw)
To: Mao Wenan
Cc: kernel-janitors, netdev, john.fastabend, hawk, jakub.kicinski,
daniel, ast, julia.lawall
Am 19.02.2019 10:06, schrieb Mao Wenan:
> This patch is to do code cleanup for ns83820_probe_phy().
> It deletes unused variable 'first' and commented out code.
>
> Signed-off-by: Mao Wenan <maowenan@huawei.com>
> ---
> v2->v3: delte unused variable 'first'; change subject from
> "net: ns83820: drop pointless static qualifier in ns83820_probe_phy()" to
> "net: ns83820: code cleanup for ns83820_probe_phy()".
> drivers/net/ethernet/natsemi/ns83820.c | 18 ------------------
> 1 file changed, 18 deletions(-)
>
> diff --git a/drivers/net/ethernet/natsemi/ns83820.c
> b/drivers/net/ethernet/natsemi/ns83820.c
> index 958fced4dacf..955d34a6f0d8 100644
> --- a/drivers/net/ethernet/natsemi/ns83820.c
> +++ b/drivers/net/ethernet/natsemi/ns83820.c
> @@ -1869,34 +1869,16 @@ static unsigned ns83820_mii_write_reg(struct ns83820
> *dev, unsigned phy, unsigne
> static void ns83820_probe_phy(struct net_device *ndev)
> {
> struct ns83820 *dev = PRIV(ndev);
> - static int first;
> int i;
> #define MII_PHYIDR1 0x02
> #define MII_PHYIDR2 0x03
>
> -#if 0
> - if (!first) {
> - unsigned tmp;
> - ns83820_mii_read_reg(dev, 1, 0x09);
> - ns83820_mii_write_reg(dev, 1, 0x10, 0x0d3e);
> -
> - tmp = ns83820_mii_read_reg(dev, 1, 0x00);
> - ns83820_mii_write_reg(dev, 1, 0x00, tmp | 0x8000);
> - udelay(1300);
> - ns83820_mii_read_reg(dev, 1, 0x09);
> - }
> -#endif
> - first = 1;
> -
> for (i=1; i<2; i++) {
the loop here seems also pointless, so you can eliminate i.
(or did i muss something ?)
just my 2 cents,
re,
wh
> int j;
> unsigned a, b;
> a = ns83820_mii_read_reg(dev, i, MII_PHYIDR1);
> b = ns83820_mii_read_reg(dev, i, MII_PHYIDR2);
>
> - //printk("%s: phy %d: 0x%04x 0x%04x\n",
> - // ndev->name, i, a, b);
> -
> for (j=0; j<0x16; j+=4) {
> dprintk("%s: [0x%02x] %04x %04x %04x %04x\n",
> ndev->name, j,
^ permalink raw reply
* Re: [PATCH v3] net: ns83820: code cleanup for ns83820_probe_phy()
From: walter harms @ 2019-02-19 12:40 UTC (permalink / raw)
To: Mao Wenan
Cc: kernel-janitors, netdev, john.fastabend, hawk, jakub.kicinski,
daniel, ast, julia.lawall
In-Reply-To: <20190219090635.134457-1-maowenan@huawei.com>
Am 19.02.2019 10:06, schrieb Mao Wenan:
> This patch is to do code cleanup for ns83820_probe_phy().
> It deletes unused variable 'first' and commented out code.
>
> Signed-off-by: Mao Wenan <maowenan@huawei.com>
> ---
> v2->v3: delte unused variable 'first'; change subject from
> "net: ns83820: drop pointless static qualifier in ns83820_probe_phy()" to
> "net: ns83820: code cleanup for ns83820_probe_phy()".
> drivers/net/ethernet/natsemi/ns83820.c | 18 ------------------
> 1 file changed, 18 deletions(-)
>
> diff --git a/drivers/net/ethernet/natsemi/ns83820.c b/drivers/net/ethernet/natsemi/ns83820.c
> index 958fced4dacf..955d34a6f0d8 100644
> --- a/drivers/net/ethernet/natsemi/ns83820.c
> +++ b/drivers/net/ethernet/natsemi/ns83820.c
> @@ -1869,34 +1869,16 @@ static unsigned ns83820_mii_write_reg(struct ns83820 *dev, unsigned phy, unsigne
> static void ns83820_probe_phy(struct net_device *ndev)
> {
> struct ns83820 *dev = PRIV(ndev);
> - static int first;
> int i;
> #define MII_PHYIDR1 0x02
> #define MII_PHYIDR2 0x03
>
> -#if 0
> - if (!first) {
> - unsigned tmp;
> - ns83820_mii_read_reg(dev, 1, 0x09);
> - ns83820_mii_write_reg(dev, 1, 0x10, 0x0d3e);
> -
> - tmp = ns83820_mii_read_reg(dev, 1, 0x00);
> - ns83820_mii_write_reg(dev, 1, 0x00, tmp | 0x8000);
> - udelay(1300);
> - ns83820_mii_read_reg(dev, 1, 0x09);
> - }
> -#endif
> - first = 1;
> -
> for (i=1; i<2; i++) {
> int j;
> unsigned a, b;
> a = ns83820_mii_read_reg(dev, i, MII_PHYIDR1);
> b = ns83820_mii_read_reg(dev, i, MII_PHYIDR2);
>
> - //printk("%s: phy %d: 0x%04x 0x%04x\n",
> - // ndev->name, i, a, b);
> -
> for (j=0; j<0x16; j+=4) {
> dprintk("%s: [0x%02x] %04x %04x %04x %04x\n",
> ndev->name, j,
^ permalink raw reply
* Re: [PATCH v3] net: ns83820: code cleanup for ns83820_probe_phy()
From: maowenan @ 2019-02-19 12:52 UTC (permalink / raw)
To: Walter Harms
Cc: kernel-janitors, netdev, john.fastabend, hawk, jakub.kicinski,
daniel, ast, julia.lawall
In-Reply-To: <715767253.151509.1550580124264@ox-groupware.bfs.de>
On 2019/2/19 20:42, Walter Harms wrote:
>
> Am 19.02.2019 10:06, schrieb Mao Wenan:
>> This patch is to do code cleanup for ns83820_probe_phy().
>> It deletes unused variable 'first' and commented out code.
>>
>> Signed-off-by: Mao Wenan <maowenan@huawei.com>
>> ---
>> v2->v3: delte unused variable 'first'; change subject from
>> "net: ns83820: drop pointless static qualifier in ns83820_probe_phy()" to
>> "net: ns83820: code cleanup for ns83820_probe_phy()".
>> drivers/net/ethernet/natsemi/ns83820.c | 18 ------------------
>> 1 file changed, 18 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/natsemi/ns83820.c
>> b/drivers/net/ethernet/natsemi/ns83820.c
>> index 958fced4dacf..955d34a6f0d8 100644
>> --- a/drivers/net/ethernet/natsemi/ns83820.c
>> +++ b/drivers/net/ethernet/natsemi/ns83820.c
>> @@ -1869,34 +1869,16 @@ static unsigned ns83820_mii_write_reg(struct ns83820
>> *dev, unsigned phy, unsigne
>> static void ns83820_probe_phy(struct net_device *ndev)
>> {
>> struct ns83820 *dev = PRIV(ndev);
>> - static int first;
>> int i;
>> #define MII_PHYIDR1 0x02
>> #define MII_PHYIDR2 0x03
>>
>> -#if 0
>> - if (!first) {
>> - unsigned tmp;
>> - ns83820_mii_read_reg(dev, 1, 0x09);
>> - ns83820_mii_write_reg(dev, 1, 0x10, 0x0d3e);
>> -
>> - tmp = ns83820_mii_read_reg(dev, 1, 0x00);
>> - ns83820_mii_write_reg(dev, 1, 0x00, tmp | 0x8000);
>> - udelay(1300);
>> - ns83820_mii_read_reg(dev, 1, 0x09);
>> - }
>> -#endif
>> - first = 1;
>> -
>> for (i=1; i<2; i++) {
>
>
> the loop here seems also pointless, so you can eliminate i.
> (or did i muss something ?)
>
good point.Thank you.
> just my 2 cents,
> re,
> wh
>> int j;
>> unsigned a, b;
>> a = ns83820_mii_read_reg(dev, i, MII_PHYIDR1);
>> b = ns83820_mii_read_reg(dev, i, MII_PHYIDR2);
>>
>> - //printk("%s: phy %d: 0x%04x 0x%04x\n",
>> - // ndev->name, i, a, b);
>> -
>> for (j=0; j<0x16; j+=4) {
>> dprintk("%s: [0x%02x] %04x %04x %04x %04x\n",
>> ndev->name, j,
>
> .
>
^ permalink raw reply
* Re: [PATCH net-next v2] net: dsa: qca8k: Enable delay for RGMII_ID mode
From: Andrew Lunn @ 2019-02-19 12:58 UTC (permalink / raw)
To: Vinod Koul
Cc: David S Miller, linux-arm-msm, Bjorn Andersson, netdev,
Niklas Cassel, Florian Fainelli, Michal Vokáč
In-Reply-To: <20190219065943.22765-1-vkoul@kernel.org>
On Tue, Feb 19, 2019 at 12:29:43PM +0530, Vinod Koul wrote:
> RGMII_ID specifies that we should have internal delay, so resurrect the
> delay addition routine but under the RGMII_ID mode.
>
> Fixes: 40269aa9f40a ("net: dsa: qca8k: disable delay for RGMII mode")
> Tested-by: Michal Vokáč <michal.vokac@ysoft.com>
> Signed-off-by: Vinod Koul <vkoul@kernel.org>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: Handling an Extra Signal at PHY Reset
From: Paul Kocialkowski @ 2019-02-19 12:53 UTC (permalink / raw)
To: Thomas Petazzoni
Cc: Andrew Lunn, Florian Fainelli, Heiner Kallweit, netdev,
Mylène Josserand
In-Reply-To: <20190219103608.56d390db@windsurf.home>
Hi,
On Tue, 2019-02-19 at 10:36 +0100, Thomas Petazzoni wrote:
> Hello Paul,
>
> On Tue, 19 Feb 2019 10:14:20 +0100
> Paul Kocialkowski <paul.kocialkowski@bootlin.com> wrote:
>
> > We are dealing with an Ethernet PHY (Marvell 88E1512) that comes with a
> > CONFIG pin that must be connected to one of the other pins of the PHY
> > to configure the LSB of the PHY address as well as I/O voltages (see
> > section 2.18.1 Hardware Configuration of the datasheet). It must be
> > connected "soon after reset" for the PHY to be correctly configured.
> >
> > We have a switch for connecting the CONFIG pin to the other pin (LED0),
> > which needs to be controlled by Linux. The CONFIG pin seems to be used
> > for a PTP clock the rest of the time.
> >
> > So we are wondering how to properly represent this case, especially on
> > the device-tree side.
> >
> > The trick here is that this step is necessary before the PHY can be
> > discovered on the MDIO bus (and thus the PHY driver selected) so we
> > can't rely on the PHY driver to do this. Basically, it looks like we
> > need to handle this like the reset pin and describe it at the MDIO bus
> > level.
> >
> > Here are some ideas for potential solutions:
> > - Allowing more than a single GPIO to be passed to the MDIO bus' reset-
> > gpios via device-tree and toggling all the passed GPIOs at once;
> >
> > - Adding a new optional GPIO for the MDIO bus dedicated to controlling
> > switches for such config switching, perhaps called "config-gpios"
> > (quite a narrow solution);
> >
> > - Adding a broader power sequence description to the MDIO bus (a bit
> > like it's done with the mmc pwrseq descriptions) which would allow
> > specifying the toggle order/delays of various GPIOs (would probably be
> > the most extensive solution);
> >
> > - Adding the extra GPIO control to the MAC description and toggling it
> > through bus->reset (probably the less invasive solution for the core
> > but not very satisfying from the description perspective, since this is
> > definitely not MAC-specific).
> >
> > What do you think about how we could solve this issue?
> > Do you see other options that I missed here?
>
> I think it's important to mention the sequence that is needed:
>
> 1. assert reset
> 2. wait 10 us
> 3. switch config signal
> 4. deassert reset
> 5. wait 100us
> 6. de-switch config signal
>
> I.e, the config signal needs to be switched properly before deasserting
> reset, and then switched back to its original state so that the config
> pin can be used for its normal (non-reset) purpose.
>
> So the manipulation of the config signal is intertwined with the
> assert/de-assert of the reset. So I don't see how your fourth option
> would work for example. Am I missing something here ?
So we discussed and checked this with Thomas and the sequence that he
mentionned is indeed the right way to go.
However, we also found that our board circuitry switches the signal
when the GPIO is not driven low. So assuming that it's either high-Z or
high when the kernel boots, we can manage with the final step (de-
switch config signal) alone (although it's safer to ensure this is the
case before reset).
Cheers,
Paul
--
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
^ permalink raw reply
* [PATCH mlx5-next] net/mlx5: Separate ODP capabilities
From: Leon Romanovsky @ 2019-02-19 13:06 UTC (permalink / raw)
To: Doug Ledford, Jason Gunthorpe
Cc: Moni Shoua, RDMA mailing list, Saeed Mahameed, linux-netdev,
Leon Romanovsky
From: Moni Shoua <monis@mellanox.com>
ODP support for XRC transport is not enabled by default in FW,
so we need separate ODP checks to enable/disable it.
While that, rewrite the set of ODP SRQ support capabilities in way
that tests each field separately for clearness, which is not needed
for current FW, but better to have it separated.
Signed-off-by: Moni Shoua <monis@mellanox.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
.../net/ethernet/mellanox/mlx5/core/main.c | 29 ++++++++++---------
1 file changed, 15 insertions(+), 14 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c b/drivers/net/ethernet/mellanox/mlx5/core/main.c
index 025dfaf3466b..5cf557a77f0f 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c
@@ -474,11 +474,6 @@ static int handle_hca_cap_odp(struct mlx5_core_dev *dev)
if (err)
return err;
- if (!(MLX5_CAP_ODP_MAX(dev, ud_odp_caps.srq_receive) ||
- MLX5_CAP_ODP_MAX(dev, rc_odp_caps.srq_receive) ||
- MLX5_CAP_ODP_MAX(dev, xrc_odp_caps.srq_receive)))
- return 0;
-
set_sz = MLX5_ST_SZ_BYTES(set_hca_cap_in);
set_ctx = kzalloc(set_sz, GFP_KERNEL);
if (!set_ctx)
@@ -488,15 +483,21 @@ static int handle_hca_cap_odp(struct mlx5_core_dev *dev)
memcpy(set_hca_cap, dev->caps.hca_cur[MLX5_CAP_ODP],
MLX5_ST_SZ_BYTES(odp_cap));
- /* set ODP SRQ support for RC/UD and XRC transports */
- MLX5_SET(odp_cap, set_hca_cap, ud_odp_caps.srq_receive,
- MLX5_CAP_ODP_MAX(dev, ud_odp_caps.srq_receive));
-
- MLX5_SET(odp_cap, set_hca_cap, rc_odp_caps.srq_receive,
- MLX5_CAP_ODP_MAX(dev, rc_odp_caps.srq_receive));
-
- MLX5_SET(odp_cap, set_hca_cap, xrc_odp_caps.srq_receive,
- MLX5_CAP_ODP_MAX(dev, xrc_odp_caps.srq_receive));
+#define ODP_CAP_SET_MAX(dev, field) \
+ do { \
+ u32 _res = MLX5_CAP_ODP_MAX(dev, field); \
+ if (_res) \
+ MLX5_SET(odp_cap, set_hca_cap, field, _res); \
+ } while (0)
+
+ ODP_CAP_SET_MAX(dev, ud_odp_caps.srq_receive);
+ ODP_CAP_SET_MAX(dev, rc_odp_caps.srq_receive);
+ ODP_CAP_SET_MAX(dev, xrc_odp_caps.srq_receive);
+ ODP_CAP_SET_MAX(dev, xrc_odp_caps.send);
+ ODP_CAP_SET_MAX(dev, xrc_odp_caps.receive);
+ ODP_CAP_SET_MAX(dev, xrc_odp_caps.write);
+ ODP_CAP_SET_MAX(dev, xrc_odp_caps.read);
+ ODP_CAP_SET_MAX(dev, xrc_odp_caps.atomic);
err = set_caps(dev, set_ctx, set_sz, MLX5_SET_HCA_CAP_OP_MOD_ODP);
--
2.19.1
^ permalink raw reply related
* Re: [RFC PATCH net-next v3 04/21] ethtool: helper functions for netlink interface
From: Michal Kubecek @ 2019-02-19 13:07 UTC (permalink / raw)
To: Jakub Kicinski
Cc: netdev, David Miller, Andrew Lunn, Jiri Pirko, linux-kernel
In-Reply-To: <20190218121517.4d299f6e@cakuba.netronome.com>
On Mon, Feb 18, 2019 at 12:15:17PM -0800, Jakub Kicinski wrote:
> On Mon, 18 Feb 2019 19:21:44 +0100 (CET), Michal Kubecek wrote:
> > +/* create skb for a reply and fill device identification
> > + * payload: payload length (without netlink and genetlink header)
> > + * dev: device the reply is about (may be null)
> > + * cmd: ETHNL_CMD_* command for reply
> > + * info: info for the received packet we respond to
> > + * ehdrp: place to store payload pointer returned by genlmsg_new()
> > + * returns: skb or null on error
> > + */
>
> nit: why not correct kdoc?
Good point. I'll fix that, together with the other things.
Michal
>
> > +struct sk_buff *ethnl_reply_init(size_t payload, struct net_device *dev, u8 cmd,
> > + u16 dev_attrtype, struct genl_info *info,
> > + void **ehdrp)
> > +{
> > + void *ehdr;
> > + struct sk_buff *rskb;
>
> nit: reverse xmas tree
^ permalink raw reply
* Re: [PATCH] phy/marvell: prevent unneeded page switching
From: Andrew Lunn @ 2019-02-19 13:19 UTC (permalink / raw)
To: Frank de Brabander; +Cc: Florian Fainelli, Heiner Kallweit, netdev
In-Reply-To: <1550560893-2445-1-git-send-email-debrabander@gmail.com>
On Tue, Feb 19, 2019 at 08:21:33AM +0100, Frank de Brabander wrote:
> marvell_read_status() no longer first switches to the fiber page, instead
> looks at the already active register page first. In case the link is down,
> switches to the other (fiber/copper) register page.
>
> Unneeded register page switching of the Marvell 88E1510 PHY can cause
> the ethernet driver to register a link down & up event.
Hi Frank
I think the page change itself is unlikely to cause a link down/up.
It seems more likely there is something else going on here. The wrong
page being read, fibre page being read when there is no support for
fibre, etc.
Do you get link down/up reported when you read the temperature sensor?
That also causes a page change?
I think i would first start by adding locking to
marvell_read_status().
phy_select_page() takes a lock before changing the page.
phy_restore_page() releases the lock. Use those around reading the
status.
Andrew
^ permalink raw reply
* [PATCH v4] net: ns83820: code cleanup for ns83820_probe_phy()
From: Mao Wenan @ 2019-02-19 13:45 UTC (permalink / raw)
To: kernel-janitors, netdev, john.fastabend, hawk, jakub.kicinski,
daniel, ast, julia.lawall, wharms
This patch is to do code cleanup for ns83820_probe_phy().
It deletes unused variable 'first', commented out code,
and the pointless 'for' loop.
Signed-off-by: Mao Wenan <maowenan@huawei.com>
Acked-by: Julia Lawall <julia.lawall@lip6.fr>
---
v3->v4: delete pointless 'for' loop, variable 'i', and so on.
drivers/net/ethernet/natsemi/ns83820.c | 70 ++++++++------------------
1 file changed, 21 insertions(+), 49 deletions(-)
diff --git a/drivers/net/ethernet/natsemi/ns83820.c b/drivers/net/ethernet/natsemi/ns83820.c
index 958fced4dacf..959c0fa37d75 100644
--- a/drivers/net/ethernet/natsemi/ns83820.c
+++ b/drivers/net/ethernet/natsemi/ns83820.c
@@ -1869,56 +1869,28 @@ static unsigned ns83820_mii_write_reg(struct ns83820 *dev, unsigned phy, unsigne
static void ns83820_probe_phy(struct net_device *ndev)
{
struct ns83820 *dev = PRIV(ndev);
- static int first;
- int i;
-#define MII_PHYIDR1 0x02
-#define MII_PHYIDR2 0x03
-
-#if 0
- if (!first) {
- unsigned tmp;
- ns83820_mii_read_reg(dev, 1, 0x09);
- ns83820_mii_write_reg(dev, 1, 0x10, 0x0d3e);
-
- tmp = ns83820_mii_read_reg(dev, 1, 0x00);
- ns83820_mii_write_reg(dev, 1, 0x00, tmp | 0x8000);
- udelay(1300);
- ns83820_mii_read_reg(dev, 1, 0x09);
- }
-#endif
- first = 1;
-
- for (i=1; i<2; i++) {
- int j;
- unsigned a, b;
- a = ns83820_mii_read_reg(dev, i, MII_PHYIDR1);
- b = ns83820_mii_read_reg(dev, i, MII_PHYIDR2);
-
- //printk("%s: phy %d: 0x%04x 0x%04x\n",
- // ndev->name, i, a, b);
-
- for (j=0; j<0x16; j+=4) {
- dprintk("%s: [0x%02x] %04x %04x %04x %04x\n",
- ndev->name, j,
- ns83820_mii_read_reg(dev, i, 0 + j),
- ns83820_mii_read_reg(dev, i, 1 + j),
- ns83820_mii_read_reg(dev, i, 2 + j),
- ns83820_mii_read_reg(dev, i, 3 + j)
- );
- }
- }
- {
- unsigned a, b;
- /* read firmware version: memory addr is 0x8402 and 0x8403 */
- ns83820_mii_write_reg(dev, 1, 0x16, 0x000d);
- ns83820_mii_write_reg(dev, 1, 0x1e, 0x810e);
- a = ns83820_mii_read_reg(dev, 1, 0x1d);
-
- ns83820_mii_write_reg(dev, 1, 0x16, 0x000d);
- ns83820_mii_write_reg(dev, 1, 0x1e, 0x810e);
- b = ns83820_mii_read_reg(dev, 1, 0x1d);
- dprintk("version: 0x%04x 0x%04x\n", a, b);
+ int j;
+ unsigned a, b;
+
+ for (j = 0; j < 0x16; j += 4) {
+ dprintk("%s: [0x%02x] %04x %04x %04x %04x\n",
+ ndev->name, j,
+ ns83820_mii_read_reg(dev, 1, 0 + j),
+ ns83820_mii_read_reg(dev, 1, 1 + j),
+ ns83820_mii_read_reg(dev, 1, 2 + j),
+ ns83820_mii_read_reg(dev, 1, 3 + j)
+ );
}
+
+ /* read firmware version: memory addr is 0x8402 and 0x8403 */
+ ns83820_mii_write_reg(dev, 1, 0x16, 0x000d);
+ ns83820_mii_write_reg(dev, 1, 0x1e, 0x810e);
+ a = ns83820_mii_read_reg(dev, 1, 0x1d);
+
+ ns83820_mii_write_reg(dev, 1, 0x16, 0x000d);
+ ns83820_mii_write_reg(dev, 1, 0x1e, 0x810e);
+ b = ns83820_mii_read_reg(dev, 1, 0x1d);
+ dprintk("version: 0x%04x 0x%04x\n", a, b);
}
#endif
--
2.20.1
^ permalink raw reply related
* Re: [Bridge] [RFC v2] net: bridge: don't flood known multicast traffic when snooping is enabled
From: Nikolay Aleksandrov @ 2019-02-19 13:31 UTC (permalink / raw)
To: Linus Lüssing; +Cc: bridge, netdev, roopa, f.fainelli, idosch
In-Reply-To: <20190219092125.GE10191@otheros>
On 19/02/2019 11:21, Linus Lüssing wrote:
> On Tue, Feb 19, 2019 at 09:57:16AM +0100, Linus Lüssing wrote:
>> On Mon, Feb 18, 2019 at 02:21:07PM +0200, Nikolay Aleksandrov wrote:
>>> This is v2 of the RFC patch which aims to forward packets to known
>>> mdsts' ports only (the no querier case). After v1 I've kept
>>> the previous behaviour when it comes to unregistered traffic or when
>>> a querier is present. All of this is of course only with snooping
>>> enabled. So with this patch the following changes should occur:
>>> - No querier: forward known mdst traffic to its registered ports,
>>> no change about unknown mcast (flood)
>>> - Querier present: no change
>>>
>>> The reason to do this is simple - we want to respect the user's mdb
>>> configuration in both cases, that is if the user adds static mdb entries
>>> manually then we should use that information about forwarding traffic.
>>>
>>> What do you think ?
>>>
>>> * Notes
>>> Traffic that is currently marked as mrouters_only:
>>> - IPv4: non-local mcast traffic, igmp reports
>>> - IPv6: non-all-nodes-dst mcast traffic, mldv1 reports
>>>
>>> Simple use case:
>>> $ echo 1 > /sys/class/net/bridge/bridge/multicast_snooping
>>> $ bridge mdb add dev bridge port swp1 grp 239.0.0.1
>>> - without a querier currently traffic for 239.0.0.1 will still be flooded,
>>> with this change it will be forwarded only to swp1
>>
>> There is still the issue with unsolicited reports adding mdst
>> entries here, too. Leading to unwanted packet loss and connectivity issues.
>
> Or in other words, an unsolicited report will turn a previously
> unregistered multicast group into a registered one. However in the
> absence of a querier the knowledge about this newly registered multicast group
> will be incomplete. And therefore still needs to be flooded to avoid packet
> loss.
>
Right, this is expected. If the user has enabled igmp snooping and doesn't have
a querier present then such behaviour is to be expected. What is surprising is
the user explicitly enabling igmp snooping, adding an mdst and then still
getting it flooded. :)
An alternative is to drop all unregistered traffic when a querier is not present.
But that will surely break setups and at best should be a configurable option that
is disabled by default.
So in effect and to try and make everybody happy we can add an option to control
this behaviour with keeping the current as default and adding the following options:
- no querier: flood all (default, current)
- no querier: flood unregistered, forward registered
- no querier: drop unregistered, forward registered
^ 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