* [PATCH net] net/ipv6: Revert attempt to simplify route replace and append
From: dsahern @ 2018-07-02 22:03 UTC (permalink / raw)
To: netdev; +Cc: idosch, sharpd, Thomas.Winter, David Ahern
From: David Ahern <dsahern@gmail.com>
NetworkManager likes to manage linklocal prefix routes and does so with
the NLM_F_APPEND flag, breaking attempts to simplify the IPv6 route
code and by extension enable multipath routes with device only nexthops.
Revert f34436a43092 and its followup
6eba08c3626b ("ipv6: Only emit append events for appended routes").
Update the test cases to reflect the old behavior.
Fixes: f34436a43092 ("net/ipv6: Simplify route replace and appending into multipath route")
Signed-off-by: David Ahern <dsahern@gmail.com>
---
Ido: I left 5a15a1b07c51. FIB_EVENT_ENTRY_APPEND is not generated for
IPv6, so no harm in leaving it.
include/net/ip6_route.h | 6 ++
net/ipv6/ip6_fib.c | 156 +++++++++++++++++--------------
net/ipv6/route.c | 3 +-
tools/testing/selftests/net/fib_tests.sh | 41 --------
4 files changed, 93 insertions(+), 113 deletions(-)
diff --git a/include/net/ip6_route.h b/include/net/ip6_route.h
index 59656fc580df..7b9c82de11cc 100644
--- a/include/net/ip6_route.h
+++ b/include/net/ip6_route.h
@@ -66,6 +66,12 @@ static inline bool rt6_need_strict(const struct in6_addr *daddr)
(IPV6_ADDR_MULTICAST | IPV6_ADDR_LINKLOCAL | IPV6_ADDR_LOOPBACK);
}
+static inline bool rt6_qualify_for_ecmp(const struct fib6_info *f6i)
+{
+ return (f6i->fib6_flags & (RTF_GATEWAY|RTF_ADDRCONF|RTF_DYNAMIC)) ==
+ RTF_GATEWAY;
+}
+
void ip6_route_input(struct sk_buff *skb);
struct dst_entry *ip6_route_input_lookup(struct net *net,
struct net_device *dev,
diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index 1fb2f3118d60..d212738e9d10 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -935,20 +935,19 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct fib6_info *rt,
{
struct fib6_info *leaf = rcu_dereference_protected(fn->leaf,
lockdep_is_held(&rt->fib6_table->tb6_lock));
- enum fib_event_type event = FIB_EVENT_ENTRY_ADD;
- struct fib6_info *iter = NULL, *match = NULL;
+ struct fib6_info *iter = NULL;
struct fib6_info __rcu **ins;
+ struct fib6_info __rcu **fallback_ins = NULL;
int replace = (info->nlh &&
(info->nlh->nlmsg_flags & NLM_F_REPLACE));
- int append = (info->nlh &&
- (info->nlh->nlmsg_flags & NLM_F_APPEND));
int add = (!info->nlh ||
(info->nlh->nlmsg_flags & NLM_F_CREATE));
int found = 0;
+ bool rt_can_ecmp = rt6_qualify_for_ecmp(rt);
u16 nlflags = NLM_F_EXCL;
int err;
- if (append)
+ if (info->nlh && (info->nlh->nlmsg_flags & NLM_F_APPEND))
nlflags |= NLM_F_APPEND;
ins = &fn->leaf;
@@ -970,8 +969,13 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct fib6_info *rt,
nlflags &= ~NLM_F_EXCL;
if (replace) {
- found++;
- break;
+ if (rt_can_ecmp == rt6_qualify_for_ecmp(iter)) {
+ found++;
+ break;
+ }
+ if (rt_can_ecmp)
+ fallback_ins = fallback_ins ?: ins;
+ goto next_iter;
}
if (rt6_duplicate_nexthop(iter, rt)) {
@@ -986,51 +990,71 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct fib6_info *rt,
fib6_metric_set(iter, RTAX_MTU, rt->fib6_pmtu);
return -EEXIST;
}
-
- /* first route that matches */
- if (!match)
- match = iter;
+ /* If we have the same destination and the same metric,
+ * but not the same gateway, then the route we try to
+ * add is sibling to this route, increment our counter
+ * of siblings, and later we will add our route to the
+ * list.
+ * Only static routes (which don't have flag
+ * RTF_EXPIRES) are used for ECMPv6.
+ *
+ * To avoid long list, we only had siblings if the
+ * route have a gateway.
+ */
+ if (rt_can_ecmp &&
+ rt6_qualify_for_ecmp(iter))
+ rt->fib6_nsiblings++;
}
if (iter->fib6_metric > rt->fib6_metric)
break;
+next_iter:
ins = &iter->fib6_next;
}
+ if (fallback_ins && !found) {
+ /* No ECMP-able route found, replace first non-ECMP one */
+ ins = fallback_ins;
+ iter = rcu_dereference_protected(*ins,
+ lockdep_is_held(&rt->fib6_table->tb6_lock));
+ found++;
+ }
+
/* Reset round-robin state, if necessary */
if (ins == &fn->leaf)
fn->rr_ptr = NULL;
/* Link this route to others same route. */
- if (append && match) {
+ if (rt->fib6_nsiblings) {
+ unsigned int fib6_nsiblings;
struct fib6_info *sibling, *temp_sibling;
- if (rt->fib6_flags & RTF_REJECT) {
- NL_SET_ERR_MSG(extack,
- "Can not append a REJECT route");
- return -EINVAL;
- } else if (match->fib6_flags & RTF_REJECT) {
- NL_SET_ERR_MSG(extack,
- "Can not append to a REJECT route");
- return -EINVAL;
+ /* Find the first route that have the same metric */
+ sibling = leaf;
+ while (sibling) {
+ if (sibling->fib6_metric == rt->fib6_metric &&
+ rt6_qualify_for_ecmp(sibling)) {
+ list_add_tail(&rt->fib6_siblings,
+ &sibling->fib6_siblings);
+ break;
+ }
+ sibling = rcu_dereference_protected(sibling->fib6_next,
+ lockdep_is_held(&rt->fib6_table->tb6_lock));
}
- event = FIB_EVENT_ENTRY_APPEND;
- rt->fib6_nsiblings = match->fib6_nsiblings;
- list_add_tail(&rt->fib6_siblings, &match->fib6_siblings);
- match->fib6_nsiblings++;
-
/* For each sibling in the list, increment the counter of
* siblings. BUG() if counters does not match, list of siblings
* is broken!
*/
+ fib6_nsiblings = 0;
list_for_each_entry_safe(sibling, temp_sibling,
- &match->fib6_siblings, fib6_siblings) {
+ &rt->fib6_siblings, fib6_siblings) {
sibling->fib6_nsiblings++;
- BUG_ON(sibling->fib6_nsiblings != match->fib6_nsiblings);
+ BUG_ON(sibling->fib6_nsiblings != rt->fib6_nsiblings);
+ fib6_nsiblings++;
}
-
- rt6_multipath_rebalance(match);
+ BUG_ON(fib6_nsiblings != rt->fib6_nsiblings);
+ rt6_multipath_rebalance(temp_sibling);
}
/*
@@ -1043,8 +1067,9 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct fib6_info *rt,
add:
nlflags |= NLM_F_CREATE;
- err = call_fib6_entry_notifiers(info->nl_net, event, rt,
- extack);
+ err = call_fib6_entry_notifiers(info->nl_net,
+ FIB_EVENT_ENTRY_ADD,
+ rt, extack);
if (err)
return err;
@@ -1062,7 +1087,7 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct fib6_info *rt,
}
} else {
- struct fib6_info *tmp;
+ int nsiblings;
if (!found) {
if (add)
@@ -1077,57 +1102,48 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct fib6_info *rt,
if (err)
return err;
- /* if route being replaced has siblings, set tmp to
- * last one, otherwise tmp is current route. this is
- * used to set fib6_next for new route
- */
- if (iter->fib6_nsiblings)
- tmp = list_last_entry(&iter->fib6_siblings,
- struct fib6_info,
- fib6_siblings);
- else
- tmp = iter;
-
- /* insert new route */
atomic_inc(&rt->fib6_ref);
rcu_assign_pointer(rt->fib6_node, fn);
- rt->fib6_next = tmp->fib6_next;
+ rt->fib6_next = iter->fib6_next;
rcu_assign_pointer(*ins, rt);
-
if (!info->skip_notify)
inet6_rt_notify(RTM_NEWROUTE, rt, info, NLM_F_REPLACE);
if (!(fn->fn_flags & RTN_RTINFO)) {
info->nl_net->ipv6.rt6_stats->fib_route_nodes++;
fn->fn_flags |= RTN_RTINFO;
}
+ nsiblings = iter->fib6_nsiblings;
+ iter->fib6_node = NULL;
+ fib6_purge_rt(iter, fn, info->nl_net);
+ if (rcu_access_pointer(fn->rr_ptr) == iter)
+ fn->rr_ptr = NULL;
+ fib6_info_release(iter);
- /* delete old route */
- rt = iter;
-
- if (rt->fib6_nsiblings) {
- struct fib6_info *tmp;
-
+ if (nsiblings) {
/* Replacing an ECMP route, remove all siblings */
- list_for_each_entry_safe(iter, tmp, &rt->fib6_siblings,
- fib6_siblings) {
- iter->fib6_node = NULL;
- fib6_purge_rt(iter, fn, info->nl_net);
- if (rcu_access_pointer(fn->rr_ptr) == iter)
- fn->rr_ptr = NULL;
- fib6_info_release(iter);
-
- rt->fib6_nsiblings--;
- info->nl_net->ipv6.rt6_stats->fib_rt_entries--;
+ ins = &rt->fib6_next;
+ iter = rcu_dereference_protected(*ins,
+ lockdep_is_held(&rt->fib6_table->tb6_lock));
+ while (iter) {
+ if (iter->fib6_metric > rt->fib6_metric)
+ break;
+ if (rt6_qualify_for_ecmp(iter)) {
+ *ins = iter->fib6_next;
+ iter->fib6_node = NULL;
+ fib6_purge_rt(iter, fn, info->nl_net);
+ if (rcu_access_pointer(fn->rr_ptr) == iter)
+ fn->rr_ptr = NULL;
+ fib6_info_release(iter);
+ nsiblings--;
+ info->nl_net->ipv6.rt6_stats->fib_rt_entries--;
+ } else {
+ ins = &iter->fib6_next;
+ }
+ iter = rcu_dereference_protected(*ins,
+ lockdep_is_held(&rt->fib6_table->tb6_lock));
}
+ WARN_ON(nsiblings != 0);
}
-
- WARN_ON(rt->fib6_nsiblings != 0);
-
- rt->fib6_node = NULL;
- fib6_purge_rt(rt, fn, info->nl_net);
- if (rcu_access_pointer(fn->rr_ptr) == rt)
- fn->rr_ptr = NULL;
- fib6_info_release(rt);
}
return 0;
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 86a0e4333d42..63f99411f0de 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -3842,7 +3842,7 @@ static struct fib6_info *rt6_multipath_first_sibling(const struct fib6_info *rt)
lockdep_is_held(&rt->fib6_table->tb6_lock));
while (iter) {
if (iter->fib6_metric == rt->fib6_metric &&
- iter->fib6_nsiblings)
+ rt6_qualify_for_ecmp(iter))
return iter;
iter = rcu_dereference_protected(iter->fib6_next,
lockdep_is_held(&rt->fib6_table->tb6_lock));
@@ -4439,7 +4439,6 @@ static int ip6_route_multipath_add(struct fib6_config *cfg,
*/
cfg->fc_nlinfo.nlh->nlmsg_flags &= ~(NLM_F_EXCL |
NLM_F_REPLACE);
- cfg->fc_nlinfo.nlh->nlmsg_flags |= NLM_F_APPEND;
nhn++;
}
diff --git a/tools/testing/selftests/net/fib_tests.sh b/tools/testing/selftests/net/fib_tests.sh
index 78245d60d8bc..0f45633bd634 100755
--- a/tools/testing/selftests/net/fib_tests.sh
+++ b/tools/testing/selftests/net/fib_tests.sh
@@ -740,13 +740,6 @@ ipv6_rt_add()
run_cmd "$IP -6 ro add unreachable 2001:db8:104::/64"
log_test $? 2 "Attempt to add duplicate route - reject route"
- # iproute2 prepend only sets NLM_F_CREATE
- # - adds a new route; does NOT convert existing route to ECMP
- add_route6 "2001:db8:104::/64" "via 2001:db8:101::2"
- run_cmd "$IP -6 ro prepend 2001:db8:104::/64 via 2001:db8:103::2"
- check_route6 "2001:db8:104::/64 via 2001:db8:101::2 dev veth1 metric 1024 2001:db8:104::/64 via 2001:db8:103::2 dev veth3 metric 1024"
- log_test $? 0 "Add new route for existing prefix (w/o NLM_F_EXCL)"
-
# route append with same prefix adds a new route
# - iproute2 sets NLM_F_CREATE | NLM_F_APPEND
add_route6 "2001:db8:104::/64" "via 2001:db8:101::2"
@@ -754,27 +747,6 @@ ipv6_rt_add()
check_route6 "2001:db8:104::/64 metric 1024 nexthop via 2001:db8:101::2 dev veth1 weight 1 nexthop via 2001:db8:103::2 dev veth3 weight 1"
log_test $? 0 "Append nexthop to existing route - gw"
- add_route6 "2001:db8:104::/64" "via 2001:db8:101::2"
- run_cmd "$IP -6 ro append 2001:db8:104::/64 dev veth3"
- check_route6 "2001:db8:104::/64 metric 1024 nexthop via 2001:db8:101::2 dev veth1 weight 1 nexthop dev veth3 weight 1"
- log_test $? 0 "Append nexthop to existing route - dev only"
-
- # multipath route can not have a nexthop that is a reject route
- add_route6 "2001:db8:104::/64" "via 2001:db8:101::2"
- run_cmd "$IP -6 ro append unreachable 2001:db8:104::/64"
- log_test $? 2 "Append nexthop to existing route - reject route"
-
- # reject route can not be converted to multipath route
- run_cmd "$IP -6 ro flush 2001:db8:104::/64"
- run_cmd "$IP -6 ro add unreachable 2001:db8:104::/64"
- run_cmd "$IP -6 ro append 2001:db8:104::/64 via 2001:db8:103::2"
- log_test $? 2 "Append nexthop to existing reject route - gw"
-
- run_cmd "$IP -6 ro flush 2001:db8:104::/64"
- run_cmd "$IP -6 ro add unreachable 2001:db8:104::/64"
- run_cmd "$IP -6 ro append 2001:db8:104::/64 dev veth3"
- log_test $? 2 "Append nexthop to existing reject route - dev only"
-
# insert mpath directly
add_route6 "2001:db8:104::/64" "nexthop via 2001:db8:101::2 nexthop via 2001:db8:103::2"
check_route6 "2001:db8:104::/64 metric 1024 nexthop via 2001:db8:101::2 dev veth1 weight 1 nexthop via 2001:db8:103::2 dev veth3 weight 1"
@@ -819,13 +791,6 @@ ipv6_rt_replace_single()
check_route6 "2001:db8:104::/64 metric 1024 nexthop via 2001:db8:101::3 dev veth1 weight 1 nexthop via 2001:db8:103::2 dev veth3 weight 1"
log_test $? 0 "Single path with multipath"
- # single path with reject
- #
- add_initial_route6 "nexthop via 2001:db8:101::2"
- run_cmd "$IP -6 ro replace unreachable 2001:db8:104::/64"
- check_route6 "unreachable 2001:db8:104::/64 dev lo metric 1024"
- log_test $? 0 "Single path with reject route"
-
# single path with single path using MULTIPATH attribute
#
add_initial_route6 "via 2001:db8:101::2"
@@ -873,12 +838,6 @@ ipv6_rt_replace_mpath()
check_route6 "2001:db8:104::/64 via 2001:db8:101::3 dev veth1 metric 1024"
log_test $? 0 "Multipath with single path via multipath attribute"
- # multipath with reject
- add_initial_route6 "nexthop via 2001:db8:101::2 nexthop via 2001:db8:103::2"
- run_cmd "$IP -6 ro replace unreachable 2001:db8:104::/64"
- check_route6 "unreachable 2001:db8:104::/64 dev lo metric 1024"
- log_test $? 0 "Multipath with reject route"
-
# route replace fails - invalid nexthop 1
add_initial_route6 "nexthop via 2001:db8:101::2 nexthop via 2001:db8:103::2"
run_cmd "$IP -6 ro replace 2001:db8:104::/64 nexthop via 2001:db8:111::3 nexthop via 2001:db8:103::3"
--
2.11.0
^ permalink raw reply related
* Re: [PATCH net-next 08/10] r8169: remove rtl8169_set_speed_xmii
From: Heiner Kallweit @ 2018-07-02 21:54 UTC (permalink / raw)
To: Andrew Lunn
Cc: David Miller, Florian Fainelli, Realtek linux nic maintainers,
netdev@vger.kernel.org
In-Reply-To: <20180702212150.GG12564@lunn.ch>
On 02.07.2018 23:21, Andrew Lunn wrote:
>> - auto_nego |= ADVERTISE_PAUSE_CAP | ADVERTISE_PAUSE_ASYM;
>
> This bit you probably want to keep. The PHY never says it support
> Pause. The MAC needs to enable pause if the MAC supports pause.
>
Actually I assumed that phylib would do this for me. But:
In phy_probe() first phydev->supported is copied to
phydev->advertising, and only after this both pause flags are added
to phydev->supported. Therefore I think they are not advertised.
Is this intentional? It sounds a little weird to me to add the
pause flags to the supported features per default, but not
advertise them.
Except e.g. we call by chance phy_set_max_speed(), which copies
phydev->supported to phydev->advertising after having adjusted
the supported speeds.
If this is not a bug, then where would be the right place to add
the pause flags to phydev->advertising?
> Andrew
>
Heiner
^ permalink raw reply
* [PATCH net-next v2 2/2] tcp: ack immediately when a cwr packet arrives
From: Lawrence Brakmo @ 2018-07-02 21:39 UTC (permalink / raw)
To: netdev
Cc: Kernel Team, Blake Matheny, Alexei Starovoitov, Neal Cardwell,
Yuchung Cheng, Steve Ibanez, Eric Dumazet
In-Reply-To: <20180702213908.1246455-1-brakmo@fb.com>
We observed high 99 and 99.9% latencies when doing RPCs with DCTCP. The
problem is triggered when the last packet of a request arrives CE
marked. The reply will carry the ECE mark causing TCP to shrink its cwnd
to 1 (because there are no packets in flight). When the 1st packet of
the next request arrives, the ACK was sometimes delayed even though it
is CWR marked, adding up to 40ms to the RPC latency.
This patch insures that CWR makred data packets arriving will be acked
immediately.
Modified based on comments by Neal Cardwell <ncardwell@google.com>
Signed-off-by: Lawrence Brakmo <brakmo@fb.com>
---
net/ipv4/tcp_input.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 76ca88f63b70..6fd1f2378f6c 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -254,10 +254,16 @@ static void tcp_ecn_withdraw_cwr(struct tcp_sock *tp)
tp->ecn_flags &= ~TCP_ECN_DEMAND_CWR;
}
-static void __tcp_ecn_check_ce(struct sock *sk, const struct sk_buff *skb)
+static void __tcp_ecn_check(struct sock *sk, const struct sk_buff *skb)
{
struct tcp_sock *tp = tcp_sk(sk);
+ /* If the sender is telling us it has entered CWR, then its cwnd may be
+ * very low (even just 1 packet), so we should ACK immediately.
+ */
+ if (tcp_hdr(skb)->cwr)
+ tcp_enter_quickack_mode(sk, 2);
+
switch (TCP_SKB_CB(skb)->ip_dsfield & INET_ECN_MASK) {
case INET_ECN_NOT_ECT:
/* Funny extension: if ECT is not set on a segment,
@@ -286,10 +292,10 @@ static void __tcp_ecn_check_ce(struct sock *sk, const struct sk_buff *skb)
}
}
-static void tcp_ecn_check_ce(struct sock *sk, const struct sk_buff *skb)
+static void tcp_ecn_check(struct sock *sk, const struct sk_buff *skb)
{
if (tcp_sk(sk)->ecn_flags & TCP_ECN_OK)
- __tcp_ecn_check_ce(sk, skb);
+ __tcp_ecn_check(sk, skb);
}
static void tcp_ecn_rcv_synack(struct tcp_sock *tp, const struct tcphdr *th)
@@ -715,7 +721,7 @@ static void tcp_event_data_recv(struct sock *sk, struct sk_buff *skb)
}
icsk->icsk_ack.lrcvtime = now;
- tcp_ecn_check_ce(sk, skb);
+ tcp_ecn_check(sk, skb);
if (skb->len >= 128)
tcp_grow_window(sk, skb);
@@ -4439,7 +4445,7 @@ static void tcp_data_queue_ofo(struct sock *sk, struct sk_buff *skb)
u32 seq, end_seq;
bool fragstolen;
- tcp_ecn_check_ce(sk, skb);
+ tcp_ecn_check(sk, skb);
if (unlikely(tcp_try_rmem_schedule(sk, skb, skb->truesize))) {
NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPOFODROP);
--
2.17.1
^ permalink raw reply related
* [PATCH net-next v2 1/2] tcp: notify when a delayed ack is sent
From: Lawrence Brakmo @ 2018-07-02 21:39 UTC (permalink / raw)
To: netdev
Cc: Kernel Team, Blake Matheny, Alexei Starovoitov, Neal Cardwell,
Yuchung Cheng, Steve Ibanez, Eric Dumazet
In-Reply-To: <20180702213908.1246455-1-brakmo@fb.com>
DCTCP depends on the CA_EVENT_NON_DELAYED_ACK and CA_EVENT_DELAYED_ACK
notifications to keep track if it needs to send an ACK for packets that
were received with a particular ECN state but whose ACK was delayed.
Under some circumstances, for example when a delayed ACK is sent with a
data packet, DCTCP state was not being updated due to a lack of
notification that the previously delayed ACK was sent. As a result, it
would sometimes send a duplicate ACK when a new data packet arrived.
This patch insures that DCTCP's state is correctly updated so it will
not send the duplicate ACK.
Improved based on comments from Neal Cardwell <ncardwell@google.com>.
Signed-off-by: Lawrence Brakmo <brakmo@fb.com>
---
net/ipv4/tcp_output.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index f8f6129160dd..acefb64e8280 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -172,6 +172,8 @@ static inline void tcp_event_ack_sent(struct sock *sk, unsigned int pkts)
__sock_put(sk);
}
tcp_dec_quickack_mode(sk, pkts);
+ if (inet_csk_ack_scheduled(sk))
+ tcp_ca_event(sk, CA_EVENT_NON_DELAYED_ACK);
inet_csk_clear_xmit_timer(sk, ICSK_TIME_DACK);
}
@@ -3567,8 +3569,6 @@ void tcp_send_ack(struct sock *sk)
if (sk->sk_state == TCP_CLOSE)
return;
- tcp_ca_event(sk, CA_EVENT_NON_DELAYED_ACK);
-
/* We are not putting this on the write queue, so
* tcp_transmit_skb() will set the ownership to this
* sock.
--
2.17.1
^ permalink raw reply related
* [PATCH net-next v2 0/2] tcp: fix high tail latencies in DCTCP
From: Lawrence Brakmo @ 2018-07-02 21:39 UTC (permalink / raw)
To: netdev
Cc: Kernel Team, Blake Matheny, Alexei Starovoitov, Neal Cardwell,
Yuchung Cheng, Steve Ibanez, Eric Dumazet
When have observed high tail latencies when using DCTCP for RPCs as
compared to using Cubic. For example, in one setup there are 2 hosts
sending to a 3rd one, with each sender having 3 flows (1 stream,
1 1MB back-to-back RPCs and 1 10KB back-to-back RPCs). The following
table shows the 99% and 99.9% latencies for both Cubic and dctcp:
Cubic 99% Cubic 99.9% dctcp 99% dctcp 99.9%
1MB RPCs 2.6ms 5.5ms 43ms 208ms
10KB RPCs 1.1ms 1.3ms 53ms 212ms
Looking at tcpdump traces showed that there are two causes for the
latency.
1) RTOs caused by the receiver sending a dup ACK and not ACKing
the last (and only) packet sent.
2) Delaying ACKs when the sender has a cwnd of 1, so everything
pauses for the duration of the delayed ACK.
The first patch fixes the cause of the dup ACKs, not updating DCTCP
state when an ACK that was initially delayed has been sent with a
data packet.
The second patch insures that an ACK is sent immediately when a
CWR marked packet arrives.
With the patches the latencies for DCTCP now look like:
dctcp 99% dctcp 99.9%
1MB RPCs 5.8ms 6.9ms
10KB RPCs 146us 203us
Note that while the 1MB RPCs tail latencies are higher than Cubic's,
the 10KB latencies are much smaller than Cubic's. These patches fix
issues on the receiver, but tcpdump traces indicate there is an
opportunity to also fix an issue at the sender that adds about 3ms
to the tail latencies.
The following trace shows the issue that tiggers an RTO (fixed by these patches):
Host A sends the last packets of the request
Host B receives them, and the last packet is marked with congestion (CE)
Host B sends ACKs for packets not marked with congestion
Host B sends data packet with reply and ACK for packet marked with
congestion (TCP flag ECE)
Host A receives ACKs with no ECE flag
Host A receives data packet with ACK for the last packet of request
and which has TCP ECE bit set
Host A sends 1st data packet of the next request with TCP flag CWR
Host B receives the packet (as seen in tcpdump at B), no CE flag
Host B sends a dup ACK that also has the TCP ECE flag
Host A RTO timer fires!
Host A to send the next packet
Host A receives an ACK for everything it has sent (i.e. Host B
did receive 1st packet of request)
Host A send more packets…
[PATCH net-next v2 1/2] tcp: notify when a delayed ack is sent
[PATCH net-next v2 2/2] tcp: ack immediately when a cwr packet
net/ipv4/tcp_input.c | 16 +++++++++++-----
net/ipv4/tcp_output.c | 4 ++--
2 files changed, 13 insertions(+), 7 deletions(-)
^ permalink raw reply
* Re: [PATCH] 6lowpan: iphc: reset mac_header after decompress to fix panic
From: Alexander Aring @ 2018-07-02 21:31 UTC (permalink / raw)
To: Michael Scott
Cc: Jukka Rissanen, David S. Miller, linux-bluetooth, linux-wpan,
netdev, linux-kernel
In-Reply-To: <20180702204346.d7bynetvzw3ayn5m@x220t>
Hi,
On Mon, Jul 02, 2018 at 04:43:46PM -0400, Alexander Aring wrote:
> Hi,
>
> On Mon, Jul 02, 2018 at 12:45:41PM -0700, Michael Scott wrote:
> > Hello Alexander,
> >
> ...
> > > Question is for me: which upper layer wants access MAC header here on
> > > receive path?
> > > It cannot parsed anyhow because so far I know no upper layer can parse
> > > at the moment 802.15.4 frames (which is a complex format). Maybe over
> > > some header_ops callback?
> >
> > I was testing a C program which performs NAT64 handling on packets
> > destined to a certain IPv6 subnet (64:ff9b::). To do this, the application
> > opens a RAW socket like this: sniff_sock = socket(PF_PACKET, SOCK_RAW,
> > htons(ETH_P_ALL)); It then sets promiscuous mode and enters a looping call
> > of:
> > length = recv(sniff_sock, buffer, PACKET_BUFFER, MSG_TRUNC); My host PC
> > kernel would then promptly crash on me. (I'm going to purposely avoid the
> > obvious point of: this probably isn't the best way to parse packets for
> > NAT64 translation as you will get every single packet incoming or outgoing
> > on the host.) Turns out, testing the program on an 802.15.4 6lowpan
> > interface exposed some of the issues which this mailing list (but not
> > myself) is well aware of (no L2 data in the RAW packets) and also led me to
> > debugging this patch to stop the kernel crash. TL;DR: To summarize, any
> > PF_PACKET SOCK_RAW socket which recv()'s IPv6 data from a 6lowpan node will
> > cause this kernel crash eventually (checked on kernel 4.15, 4.16, 4.17 and
> > 4.18-rc1). - Mike
> > >
>
> "any PF_PACKET SOCK_RAW" can't be otherwise I would also see it with my
> sniffer programs e.g. wireshark or tcpdump which use libpcap.
>
> There need to be some different in the handling. This is what I have
> currently in my mind.
>
> I currently not sure how to set skb->mac_header if interface is RAW_IP.
> It seems there is an indicator that mac header is not set. Example:
>
> diff --git a/net/6lowpan/iphc.c b/net/6lowpan/iphc.c
> index 6b1042e21656..e6ec2df3afe0 100644
> --- a/net/6lowpan/iphc.c
> +++ b/net/6lowpan/iphc.c
> @@ -770,6 +770,7 @@ int lowpan_header_decompress(struct sk_buff *skb, const struct net_device *dev,
> hdr.hop_limit, &hdr.daddr);
>
> skb_push(skb, sizeof(hdr));
> + skb->mac_header = (typeof(skb->mac_header))~0U;
> skb_reset_network_header(skb);
> skb_copy_to_linear_data(skb, &hdr, sizeof(hdr));
>
>
> Maybe we should lookup what skb->mac_header points to on tun interfaces
> then do the same.
So far I see [0] tun does the same as your patch approach does. network
header and mac header points to the same.
I think then we should go with your patch.
Then we just need to solve the issue to get mac header information on
top of lowpan socket layer... out of scope issue but indeed we need to
solve that. As we talked there exists even UDP protocols which needs mac
header information. It's in a pretty early state, I have no idea how
this fits sometimes with fragmentation handling together. At least for
L2 address handling and fragmentation it should be fine (one of the
fragment indentifier).
- Alex
[0] https://elixir.bootlin.com/linux/v4.18-rc3/source/drivers/net/tun.c#L1912
^ permalink raw reply
* Re: [PATCH net-next 07/10] r8169: migrate speed_down function to phylib
From: Heiner Kallweit @ 2018-07-02 21:31 UTC (permalink / raw)
To: Andrew Lunn
Cc: David Miller, Florian Fainelli, Realtek linux nic maintainers,
netdev@vger.kernel.org
In-Reply-To: <20180702212005.GF12564@lunn.ch>
On 02.07.2018 23:20, Andrew Lunn wrote:
> On Mon, Jul 02, 2018 at 09:37:08PM +0200, Heiner Kallweit wrote:
>> Change rtl_speed_down() to use phylib.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>> drivers/net/ethernet/realtek/r8169.c | 33 +++++++++++++---------------
>> 1 file changed, 15 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
>> index 311321ee..807fbc75 100644
>> --- a/drivers/net/ethernet/realtek/r8169.c
>> +++ b/drivers/net/ethernet/realtek/r8169.c
>> @@ -4240,6 +4240,10 @@ static void rtl8169_init_phy(struct net_device *dev, struct rtl8169_private *tp)
>> rtl_writephy(tp, 0x0b, 0x0000); //w 0x0b 15 0 0
>> }
>>
>> + /* We may have called rtl_speed_down before */
>> + dev->phydev->advertising = dev->phydev->supported;
>> + genphy_config_aneg(dev->phydev);
>> +
>> genphy_soft_reset(dev->phydev);
>>
>> rtl8169_set_speed(dev, AUTONEG_ENABLE, SPEED_1000, DUPLEX_FULL,
>> @@ -4323,28 +4327,21 @@ static void rtl_init_mdio_ops(struct rtl8169_private *tp)
>> }
>> }
>>
>> +#define BASET10 (ADVERTISED_10baseT_Half | ADVERTISED_10baseT_Full)
>> +#define BASET100 (ADVERTISED_100baseT_Half | ADVERTISED_100baseT_Full)
>> +#define BASET1000 (ADVERTISED_1000baseT_Half | ADVERTISED_1000baseT_Full)
>> +
>> static void rtl_speed_down(struct rtl8169_private *tp)
>> {
>> - u32 adv;
>> - int lpa;
>> + struct phy_device *phydev = tp->dev->phydev;
>> + u32 adv = phydev->lp_advertising & phydev->supported;
>>
>> - rtl_writephy(tp, 0x1f, 0x0000);
>> - lpa = rtl_readphy(tp, MII_LPA);
>> + if (adv & BASET10)
>> + phydev->advertising &= ~(BASET100 | BASET1000);
>> + else if (adv & BASET100)
>> + phydev->advertising &= ~BASET1000;
>>
>> - if (lpa & (LPA_10HALF | LPA_10FULL))
>> - adv = ADVERTISED_10baseT_Half | ADVERTISED_10baseT_Full;
>> - else if (lpa & (LPA_100HALF | LPA_100FULL))
>> - adv = ADVERTISED_10baseT_Half | ADVERTISED_10baseT_Full |
>> - ADVERTISED_100baseT_Half | ADVERTISED_100baseT_Full;
>> - else
>> - adv = ADVERTISED_10baseT_Half | ADVERTISED_10baseT_Full |
>> - ADVERTISED_100baseT_Half | ADVERTISED_100baseT_Full |
>> - (tp->mii.supports_gmii ?
>> - ADVERTISED_1000baseT_Half |
>> - ADVERTISED_1000baseT_Full : 0);
>> -
>> - rtl8169_set_speed(tp->dev, AUTONEG_ENABLE, SPEED_1000, DUPLEX_FULL,
>> - adv);
>> + genphy_config_aneg(phydev);
>> }
>
> It probably it is me being too tired, but i don't get what this is
> doing? Changing the local advertisement based on what the remote is
> advertising. Why?
>
It also took me some time to understand what this speed_down is doing.
If we suspend and wait for a WoL packet, then we don't have to burn all
the energy for a GBit connection. Therefore we switch to the lowest
speed supported by chip and link partner. This is done by removing
higher speeds from the advertised modes and restarting an autonego.
> Andrew
>
^ permalink raw reply
* Re: [PATCH net-next 02/10] r8169: use phy_resume/phy_suspend
From: Heiner Kallweit @ 2018-07-02 21:24 UTC (permalink / raw)
To: Andrew Lunn
Cc: David Miller, Florian Fainelli, Realtek linux nic maintainers,
netdev@vger.kernel.org
In-Reply-To: <20180702210646.GB12564@lunn.ch>
On 02.07.2018 23:06, Andrew Lunn wrote:
>> static void r8168_pll_power_down(struct rtl8169_private *tp)
>> {
>> if (r8168_check_dash(tp))
>> @@ -4510,7 +4469,8 @@ static void r8168_pll_power_down(struct rtl8169_private *tp)
>> if (rtl_wol_pll_power_down(tp))
>> return;
>>
>> - r8168_phy_power_down(tp);
>> + /* cover the case that PHY isn't connected */
>> + phy_suspend(mdiobus_get_phy(tp->mii_bus, 0));
>
> This could do some more explanation. Why would it not be connected?
>
The PHY gets connected when the net_device is opened. If a network
port isn't used then it will be runtime-suspended a few seconds after
boot. In this case we call r8168_pll_power_down() with the PHY not
being connected.
> Andrew
>
^ permalink raw reply
* Re: [PATCH net-next 01/10] r8169: add basic phylib support
From: Heiner Kallweit @ 2018-07-02 21:15 UTC (permalink / raw)
To: Andrew Lunn
Cc: David Miller, Florian Fainelli, Realtek linux nic maintainers,
netdev@vger.kernel.org
In-Reply-To: <20180702210239.GA12564@lunn.ch>
On 02.07.2018 23:02, Andrew Lunn wrote:
>> +static int r8169_mdio_read_reg(struct mii_bus *mii_bus, int phyaddr, int phyreg)
>> +{
>> + struct rtl8169_private *tp = mii_bus->priv;
>> +
>> + return rtl_readphy(tp, phyreg);
>
> So there is no support for phyaddr?
>
Right, the chip can access only the one internal PHY, therefore it
doesn't support phyaddr.
> It would be better to trap the phyaddr which are not supported and
> return 0xffff.
>
OK
> Andrew
>
^ permalink raw reply
* Re: [PATCH net-next 2/2] tcp: ack immediately when a cwr packet arrives
From: Lawrence Brakmo @ 2018-07-02 21:24 UTC (permalink / raw)
To: Neal Cardwell
Cc: Netdev, Kernel Team, Blake Matheny, Alexei Starovoitov,
Yuchung Cheng, Steve Ibanez, Eric Dumazet
In-Reply-To: <CADVnQy=3s4AAX-SyHaRURPgA=mN=m4DBP=Jz3HGt5pBPbiRaaA@mail.gmail.com>
On 7/2/18, 7:57 AM, "Neal Cardwell" <ncardwell@google.com> wrote:
On Sat, Jun 30, 2018 at 9:47 PM Lawrence Brakmo <brakmo@fb.com> wrote:
> I see two issues, one is that entering quickack mode as you
> mentioned does not insure that it will still be on when the CWR
> arrives. The second issue is that the problem occurs right after the
> receiver sends a small reply which results in entering pingpong mode
> right before the sender starts the new request by sending just one
> packet (i.e. forces delayed ack).
>
> I compiled and tested your patch. Both 99 and 99.9 percentile
> latencies are around 40ms. Looking at the packet traces shows that
> some CWR marked packets are not being ack immediately (delayed by
> 40ms).
Thanks, Larry! So your tests provide nice, specific evidence that it
is good to force an immediate ACK when a receiver receives a packet
with CWR marked. Given that, I am wondering what the simplest way is
to achieve that goal.
What if, rather than plumbing a new specific signal into
__tcp_ack_snd_check(), we use the existing general quick-ack
mechanism, where various parts of the TCP stack (like
__tcp_ecn_check_ce()) are already using the quick-ack mechanism to
"remotely" signal to __tcp_ack_snd_check() that they want an immediate
ACK.
For example, would it work to do something like:
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index c53ae5fc834a5..8168d1938b376 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -262,6 +262,12 @@ static void __tcp_ecn_check_ce(struct sock *sk,
const struct sk_buff *skb)
{
struct tcp_sock *tp = tcp_sk(sk);
+ /* If the sender is telling us it has entered CWR, then its cwnd may be
+ * very low (even just 1 packet), so we should ACK immediately.
+ */
+ if (tcp_hdr(skb)->cwr)
+ tcp_enter_quickack_mode(sk, 2);
+
switch (TCP_SKB_CB(skb)->ip_dsfield & INET_ECN_MASK) {
case INET_ECN_NOT_ECT:
/* Insure that GCN will not continue to mark packets. */
And then since that broadens the mission of this function beyond
checking just the ECT/CE bits, I supposed we could rename the
__tcp_ecn_check_ce() and tcp_ecn_check_ce() functions to
__tcp_ecn_check() and tcp_ecn_check(), or something like that.
Would that work for this particular issue?
Neal
Thanks Neal, it does work and is cleaner than what I was doing. I will submit a revised patch set.
^ permalink raw reply
* Re: [PATCH net-next 1/2] tcp: notify when a delayed ack is sent
From: Lawrence Brakmo @ 2018-07-02 21:24 UTC (permalink / raw)
To: Neal Cardwell
Cc: Netdev, Kernel Team, Blake Matheny, Alexei Starovoitov,
Yuchung Cheng, Steve Ibanez, Eric Dumazet
In-Reply-To: <CADVnQykHXktzKY_jXa9GRwPciTf-1CsAegGRPyUc+F1mdMMmBw@mail.gmail.com>
On 7/2/18, 8:18 AM, "netdev-owner@vger.kernel.org on behalf of Neal Cardwell" <netdev-owner@vger.kernel.org on behalf of ncardwell@google.com> wrote:
On Fri, Jun 29, 2018 at 9:48 PM Lawrence Brakmo <brakmo@fb.com> wrote:
>
> DCTCP depends on the CA_EVENT_NON_DELAYED_ACK and CA_EVENT_DELAYED_ACK
> notifications to keep track if it needs to send an ACK for packets that
> were received with a particular ECN state but whose ACK was delayed.
>
> Under some circumstances, for example when a delayed ACK is sent with a
> data packet, DCTCP state was not being updated due to a lack of
> notification that the previously delayed ACK was sent. As a result, it
> would sometimes send a duplicate ACK when a new data packet arrived.
>
> This patch insures that DCTCP's state is correctly updated so it will
> not send the duplicate ACK.
>
> Signed-off-by: Lawrence Brakmo <brakmo@fb.com>
> ---
> net/ipv4/tcp_output.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index f8f6129160dd..41f6ad7a21e4 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -172,6 +172,8 @@ static inline void tcp_event_ack_sent(struct sock *sk, unsigned int pkts)
> __sock_put(sk);
> }
> tcp_dec_quickack_mode(sk, pkts);
> + if (inet_csk_ack_scheduled(sk))
> + tcp_ca_event(sk, CA_EVENT_NON_DELAYED_ACK);
> inet_csk_clear_xmit_timer(sk, ICSK_TIME_DACK);
> }
Thanks for this fix! Seems like this would work, but if I am reading
this correctly then it seems like this would cause a duplicate call to
tcp_ca_event(sk, CA_EVENT_NON_DELAYED_ACK) when we are sending a pure
ACK (delayed or non-delayed):
(1) once from tcp_send_ack() before we send the ACK:
tcp_send_ack(struct sock *sk)
-> tcp_ca_event(sk, CA_EVENT_NON_DELAYED_ACK);
(2) then again from tcp_event_ack_sent() after we have sent the ACK:
tcp_event_ack_sent()
-> if (inet_csk_ack_scheduled(sk))
tcp_ca_event(sk, CA_EVENT_NON_DELAYED_ACK);
What if we remove the original CA_EVENT_NON_DELAYED_ACK call and just
replace it with your new one? (not compiled, not tested):
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 3889dcd4868d4..bddb49617d9be 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -184,6 +184,8 @@ static inline void tcp_event_ack_sent(struct sock
*sk, unsigned int pkts)
__sock_put(sk);
}
tcp_dec_quickack_mode(sk, pkts);
+ if (inet_csk_ack_scheduled(sk))
+ tcp_ca_event(sk, CA_EVENT_NON_DELAYED_ACK);
inet_csk_clear_xmit_timer(sk, ICSK_TIME_DACK);
}
@@ -3836,8 +3838,6 @@ void tcp_send_ack(struct sock *sk)
if (sk->sk_state == TCP_CLOSE)
return;
- tcp_ca_event(sk, CA_EVENT_NON_DELAYED_ACK);
-
/* We are not putting this on the write queue, so
* tcp_transmit_skb() will set the ownership to this
* sock.
Aside from lower CPU overhead, one nice benefit of that is that we
then only call tcp_ca_event(sk, CA_EVENT_NON_DELAYED_ACK) in one
place, which might be a little easier to reason about.
Does that work?
neal
Thanks Neal, good catch! I will resubmit the patch.
^ permalink raw reply
* Re: [PATCH net-next 0/7] net/ipv6: Fix route append and replace use cases
From: David Ahern @ 2018-07-02 21:23 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Thomas.Winter, idosch, sharpd, roopa
In-Reply-To: <6d61c529-5b51-dc3b-86ee-e912f1a8e0a8@gmail.com>
On 5/22/18 2:44 PM, David Ahern wrote:
> On 5/22/18 12:46 PM, David Miller wrote:
>>
>> Ok, I'll apply this series.
>>
>> But if this breaks things for anyone in a practical way, I am unfortunately
>> going to have to revert no matter how silly the current behavior may be.
>>
>
> Understood. I have to try the best option first. I'll look at
> regressions if they happen.
>
Debugging the problem Sowmini reported and I discovered NetworkManager
likes to manage link-local prefix routes (fe80::/64), and it likes to do
so with NLM_F_APPEND so the routes end up like this:
fe80::/64 dev eth0 proto kernel metric 100 pref medium
fe80::/64 dev eth1 proto kernel metric 101 pref medium
fe80::/64 proto kernel metric 256
nexthop dev eth1 weight 1
nexthop dev eth0 weight 1 pref medium
NM deletes the kernel installed routes, and then adds new ones -- twice:
once with the custom metric and then re-installs the route it deleted
but with the append flag.
Given the short amount of time left for 4.18 (and summer PTO), I think a
revert is the only option; we can try again in a future release.
^ permalink raw reply
* Re: [PATCH net-next 08/10] r8169: remove rtl8169_set_speed_xmii
From: Andrew Lunn @ 2018-07-02 21:21 UTC (permalink / raw)
To: Heiner Kallweit
Cc: David Miller, Florian Fainelli, Realtek linux nic maintainers,
netdev@vger.kernel.org
In-Reply-To: <fb72a450-8f15-2f34-38ae-26c7c3a5d159@gmail.com>
> - auto_nego |= ADVERTISE_PAUSE_CAP | ADVERTISE_PAUSE_ASYM;
This bit you probably want to keep. The PHY never says it support
Pause. The MAC needs to enable pause if the MAC supports pause.
Andrew
^ permalink raw reply
* Re: [PATCH net-next 07/10] r8169: migrate speed_down function to phylib
From: Andrew Lunn @ 2018-07-02 21:20 UTC (permalink / raw)
To: Heiner Kallweit
Cc: David Miller, Florian Fainelli, Realtek linux nic maintainers,
netdev@vger.kernel.org
In-Reply-To: <5be3d523-bb26-c2d6-71f4-165e9c6e45a9@gmail.com>
On Mon, Jul 02, 2018 at 09:37:08PM +0200, Heiner Kallweit wrote:
> Change rtl_speed_down() to use phylib.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
> drivers/net/ethernet/realtek/r8169.c | 33 +++++++++++++---------------
> 1 file changed, 15 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
> index 311321ee..807fbc75 100644
> --- a/drivers/net/ethernet/realtek/r8169.c
> +++ b/drivers/net/ethernet/realtek/r8169.c
> @@ -4240,6 +4240,10 @@ static void rtl8169_init_phy(struct net_device *dev, struct rtl8169_private *tp)
> rtl_writephy(tp, 0x0b, 0x0000); //w 0x0b 15 0 0
> }
>
> + /* We may have called rtl_speed_down before */
> + dev->phydev->advertising = dev->phydev->supported;
> + genphy_config_aneg(dev->phydev);
> +
> genphy_soft_reset(dev->phydev);
>
> rtl8169_set_speed(dev, AUTONEG_ENABLE, SPEED_1000, DUPLEX_FULL,
> @@ -4323,28 +4327,21 @@ static void rtl_init_mdio_ops(struct rtl8169_private *tp)
> }
> }
>
> +#define BASET10 (ADVERTISED_10baseT_Half | ADVERTISED_10baseT_Full)
> +#define BASET100 (ADVERTISED_100baseT_Half | ADVERTISED_100baseT_Full)
> +#define BASET1000 (ADVERTISED_1000baseT_Half | ADVERTISED_1000baseT_Full)
> +
> static void rtl_speed_down(struct rtl8169_private *tp)
> {
> - u32 adv;
> - int lpa;
> + struct phy_device *phydev = tp->dev->phydev;
> + u32 adv = phydev->lp_advertising & phydev->supported;
>
> - rtl_writephy(tp, 0x1f, 0x0000);
> - lpa = rtl_readphy(tp, MII_LPA);
> + if (adv & BASET10)
> + phydev->advertising &= ~(BASET100 | BASET1000);
> + else if (adv & BASET100)
> + phydev->advertising &= ~BASET1000;
>
> - if (lpa & (LPA_10HALF | LPA_10FULL))
> - adv = ADVERTISED_10baseT_Half | ADVERTISED_10baseT_Full;
> - else if (lpa & (LPA_100HALF | LPA_100FULL))
> - adv = ADVERTISED_10baseT_Half | ADVERTISED_10baseT_Full |
> - ADVERTISED_100baseT_Half | ADVERTISED_100baseT_Full;
> - else
> - adv = ADVERTISED_10baseT_Half | ADVERTISED_10baseT_Full |
> - ADVERTISED_100baseT_Half | ADVERTISED_100baseT_Full |
> - (tp->mii.supports_gmii ?
> - ADVERTISED_1000baseT_Half |
> - ADVERTISED_1000baseT_Full : 0);
> -
> - rtl8169_set_speed(tp->dev, AUTONEG_ENABLE, SPEED_1000, DUPLEX_FULL,
> - adv);
> + genphy_config_aneg(phydev);
> }
It probably it is me being too tired, but i don't get what this is
doing? Changing the local advertisement based on what the remote is
advertising. Why?
Andrew
^ permalink raw reply
* Re: [PATCH net-next 06/10] r8169: use phy_mii_ioctl
From: Andrew Lunn @ 2018-07-02 21:13 UTC (permalink / raw)
To: Heiner Kallweit
Cc: David Miller, Florian Fainelli, Realtek linux nic maintainers,
netdev@vger.kernel.org
In-Reply-To: <9da55dc3-6e96-14e2-ac1c-1d621da533fe@gmail.com>
On Mon, Jul 02, 2018 at 09:37:05PM +0200, Heiner Kallweit wrote:
> Switch to using phy_mii_ioctl().
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH bpf 3/3] bpf: undo prog rejection on read-only lock failure
From: Daniel Borkmann @ 2018-07-02 21:12 UTC (permalink / raw)
To: Kees Cook; +Cc: Alexei Starovoitov, Network Development, Laura Abbott
In-Reply-To: <CAGXu5jLCFhxw2MVg4Gfz=yrYzxONr0ESHtePPeS=xgM3mQUhsQ@mail.gmail.com>
On 07/02/2018 08:48 PM, Kees Cook wrote:
> On Fri, Jun 29, 2018 at 4:47 PM, Daniel Borkmann <daniel@iogearbox.net> wrote:
>> On 06/29/2018 08:42 PM, Kees Cook wrote:
>>> On Thu, Jun 28, 2018 at 2:34 PM, Daniel Borkmann <daniel@iogearbox.net> wrote:
>>>> Kees suggested that if set_memory_*() can fail, we should annotate it with
>>>> __must_check, and all callers need to deal with it gracefully given those
>>>> set_memory_*() markings aren't "advisory", but they're expected to actually
>>>> do what they say. This might be an option worth to move forward in future
>>>> but would at the same time require that set_memory_*() calls from supporting
>>>> archs are guaranteed to be "atomic" in that they provide rollback if part
>>>> of the range fails, once that happened, the transition from RW -> RO could
>>>> be made more robust that way, while subsequent RO -> RW transition /must/
>>>> continue guaranteeing to always succeed the undo part.
>>>
>>> Does this mean we can have BPF filters that aren't read-only then?
>>> What's the situation where set_memory_ro() fails? (Can it be induced
>>> by the user?)
>>
>> My understanding is that the cpa_process_alias() would attempt to also change
>> attributes of physmap ranges, and it found that a large page had to be split
>> for this but failed in doing so thus attributes couldn't be updated there due
>> to page alloc error. Attempting to change the primary mapping which would be
>> directly the addr passed to set_memory_ro() was however set to read-only
>> despite error. While for reproduction I had a toggle on the alloc_pages() in
>> split_large_page() to have it fail, I only could trigger it occasionally; I
>> used the selftest suite in a loop to stress test and it hit about or twice
>> over hours.
>
> Okay, so it's pretty rare; that's good! :P
>
> It really seems like this should be a situation that never fails, but
> if we ARE going to allow failures, then I think we need to propagate
> them up to callers. That means modules could fail to load in these
> cases, etc, etc. Since this is a fundamental protection, we need to
> either never fail to set things RO or we need to disallow operation
> continuing in the face of something NOT being RO.
Yeah, fully agree with you, and set_memory_*() would need to be reworked for the
archs implementing them to recover from failure and rollback any attribute changes
already done from the call. Today it's not the case, so this would be first step,
and second step to add the checks for error and bail out from various call-sites.
Thanks,
Daniel
^ permalink raw reply
* Re: [PATCH net-next 05/10] r8169: use phy_ethtool_nway_reset
From: Andrew Lunn @ 2018-07-02 21:11 UTC (permalink / raw)
To: Heiner Kallweit
Cc: David Miller, Florian Fainelli, Realtek linux nic maintainers,
netdev@vger.kernel.org
In-Reply-To: <968b2379-9201-d4e5-502c-566dfef1a1ea@gmail.com>
On Mon, Jul 02, 2018 at 09:37:03PM +0200, Heiner Kallweit wrote:
> Switch to using phy_ethtool_nway_reset().
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH net-next 03/10] r8169: replace open-coded PHY soft reset with genphy_soft_reset
From: Andrew Lunn @ 2018-07-02 21:08 UTC (permalink / raw)
To: Heiner Kallweit
Cc: David Miller, Florian Fainelli, Realtek linux nic maintainers,
netdev@vger.kernel.org
In-Reply-To: <ac6d5745-2c4c-d229-12f3-b6b45b95b8ed@gmail.com>
On Mon, Jul 02, 2018 at 09:36:58PM +0200, Heiner Kallweit wrote:
> Use genphy_soft_reset() instead of open-coding a PHY soft reset. We have
> to do an explicit PHY soft reset because some chips use the genphy driver
> which uses a no-op as soft_reset callback.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH net-next 02/10] r8169: use phy_resume/phy_suspend
From: Andrew Lunn @ 2018-07-02 21:06 UTC (permalink / raw)
To: Heiner Kallweit
Cc: David Miller, Florian Fainelli, Realtek linux nic maintainers,
netdev@vger.kernel.org
In-Reply-To: <318ae669-d89c-636d-94ac-d4c9f662400e@gmail.com>
> static void r8168_pll_power_down(struct rtl8169_private *tp)
> {
> if (r8168_check_dash(tp))
> @@ -4510,7 +4469,8 @@ static void r8168_pll_power_down(struct rtl8169_private *tp)
> if (rtl_wol_pll_power_down(tp))
> return;
>
> - r8168_phy_power_down(tp);
> + /* cover the case that PHY isn't connected */
> + phy_suspend(mdiobus_get_phy(tp->mii_bus, 0));
This could do some more explanation. Why would it not be connected?
Andrew
^ permalink raw reply
* Re: [PATCH net-next 01/10] r8169: add basic phylib support
From: Andrew Lunn @ 2018-07-02 21:02 UTC (permalink / raw)
To: Heiner Kallweit
Cc: David Miller, Florian Fainelli, Realtek linux nic maintainers,
netdev@vger.kernel.org
In-Reply-To: <60049e7e-b86d-1968-cdfd-7e0f91a25b88@gmail.com>
> +static int r8169_mdio_read_reg(struct mii_bus *mii_bus, int phyaddr, int phyreg)
> +{
> + struct rtl8169_private *tp = mii_bus->priv;
> +
> + return rtl_readphy(tp, phyreg);
So there is no support for phyaddr?
It would be better to trap the phyaddr which are not supported and
return 0xffff.
Andrew
^ permalink raw reply
* [PATCH] gen_stats: Fix netlink stats dumping in the presence of padding
From: Toke Høiland-Jørgensen @ 2018-07-02 20:52 UTC (permalink / raw)
To: netdev; +Cc: cake
The gen_stats facility will add a header for the toplevel nlattr of type
TCA_STATS2 that contains all stats added by qdisc callbacks. A reference
to this header is stored in the gnet_dump struct, and when all the
per-qdisc callbacks have finished adding their stats, the length of the
containing header will be adjusted to the right value.
However, on architectures that need padding (i.e., that don't set
CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS), the padding nlattr is added
before the stats, which means that the stored pointer will point to the
padding, and so when the header is fixed up, the result is just a very
big padding nlattr. Because most qdiscs also supply the legacy TCA_STATS
struct, this problem has been mostly invisible, but we exposed it with
the netlink attribute-based statistics in CAKE.
Fix the issue by fixing up the stored pointer if it points to a padding
nlattr.
Tested-by: Pete Heist <pete@heistp.net>
Tested-by: Kevin Darbyshire-Bryant <kevin@darbyshire-bryant.me.uk>
Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
---
net/core/gen_stats.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/net/core/gen_stats.c b/net/core/gen_stats.c
index b2b2323bdc84..188d693cb251 100644
--- a/net/core/gen_stats.c
+++ b/net/core/gen_stats.c
@@ -77,8 +77,20 @@ gnet_stats_start_copy_compat(struct sk_buff *skb, int type, int tc_stats_type,
d->lock = lock;
spin_lock_bh(lock);
}
- if (d->tail)
- return gnet_stats_copy(d, type, NULL, 0, padattr);
+ if (d->tail) {
+ int ret = gnet_stats_copy(d, type, NULL, 0, padattr);
+
+ /* The initial attribute added in gnet_stats_copy() may be
+ * preceded by a padding attribute, in which case d->tail will
+ * end up pointing at the padding instead of the real attribute.
+ * Fix this so gnet_stats_finish_copy() adjusts the length of
+ * the right attribute.
+ */
+ if (ret == 0 && d->tail->nla_type == padattr)
+ d->tail = (struct nlattr *)((char *)d->tail +
+ NLA_ALIGN(d->tail->nla_len));
+ return ret;
+ }
return 0;
}
^ permalink raw reply related
* [PATCH net] r8169: fix mac address change
From: Heiner Kallweit @ 2018-07-02 20:49 UTC (permalink / raw)
To: David Miller, Corinna Vinschen, netdev@vger.kernel.org
Network core refuses to change mac address because flag
IFF_LIVE_ADDR_CHANGE isn't set. Set this missing flag.
Fixes: 1f7aa2bc268e ("r8169: simplify rtl_set_mac_address")
Reported-by: Corinna Vinschen <vinschen@redhat.com>
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/ethernet/realtek/r8169.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index f80ac894..a390db27 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -7607,6 +7607,7 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
NETIF_F_HW_VLAN_CTAG_RX;
dev->vlan_features = NETIF_F_SG | NETIF_F_IP_CSUM | NETIF_F_TSO |
NETIF_F_HIGHDMA;
+ dev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
tp->cp_cmd |= RxChkSum | RxVlan;
--
2.18.0
^ permalink raw reply related
* RE: [RFC net-next 15/15] net: lora: Add Semtech SX1301
From: Ben Whitten @ 2018-07-02 20:43 UTC (permalink / raw)
To: Andreas Färber, Mark Brown
Cc: netdev@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, Jian-Hong Pan, Jiri Pirko,
Marcel Holtmann, David S . Miller, Matthias Brugger, Janus Piwek,
Michael Röder, Dollar Chen, Ken Yu, Steve deRosier,
linux-spi@vger.kernel.org, LoRa_Community_Support@semtech.com
In-Reply-To: <4e06cc72-2092-70f3-c801-bf6e4c3cbec2@suse.de>
> Subject: Re: [RFC net-next 15/15] net: lora: Add Semtech SX1301
>
> Hi Mark,
>
> This driver is still evolving, there's newer code on my lora-next branch
> already: https://github.com/afaerber/linux/commits/lora-next
>
> The reason you're in CC on this RFC is two-fold:
>
> 1) You applied Ben's patch to associate "semtech,sx1301" with spidev,
> whereas I am now preparing a new driver for the same compatible.
>
> 2) This SPI device is in turn exposing the two SPI masters that you
> already found below, and I didn't see a sane way to split that code out
> into drivers/spi/, so it's in drivers/net/lora/ here - has there been
> any precedence either way?
In my work in progress driver I just register one controller for the sx1301 with two chip selects and use the chip select information to choose the correct radio to send to, this is based on the DT reg information. No need to register two separate masters.
> More inline ...
>
> Am 02.07.2018 um 18:12 schrieb Mark Brown:
> > On Sun, Jul 01, 2018 at 01:08:04PM +0200, Andreas Färber wrote:
> >
> >> +static void sx1301_radio_spi_set_cs(struct spi_device *spi, bool enable)
> >> +{
> >> + int ret;
> >> +
> >> + dev_dbg(&spi->dev, "setting SPI CS to %s\n", enable ? "1" : "0");
> >> +
> >> + if (enable)
> >> + return;
> >> +
> >> + ret = sx1301_radio_set_cs(spi->controller, enable);
> >> + if (ret)
> >> + dev_warn(&spi->dev, "failed to write CS (%d)\n", ret);
> >> +}
> >
> > So we never disable chip select?
>
> Not here, I instead did that in transfer_one below.
>
> Unfortunately there seems to be no documentation, only reference code:
>
> https://github.com/Lora-
> net/lora_gateway/blob/master/libloragw/src/loragw_radio.c#L121
> https://github.com/Lora-
> net/lora_gateway/blob/master/libloragw/src/loragw_radio.c#L165
>
> It sets CS to 0 before writing to address and data registers, then
> immediately sets CS to 1 and back to 0 before reading or ending the
> write transaction. I've tried to force the same behavior in this driver.
> My guess was that CS is high-active during the short 1-0 cycle, because
> if it's low-active during the register writes then why the heck is it
> set to 0 again in the end instead of keeping at 1... confusing.
>
> Maybe the Semtech folks CC'ed can comment how these registers work?
>
> >> + if (tx_buf) {
> >> + ret = sx1301_write(ssx->parent, ssx->regs +
> REG_RADIO_X_ADDR, tx_buf ? tx_buf[0] : 0);
> >
> > This looks confused. We're in an if (tx_buf) block but there's a use of
> > the ternery operator that appears to be checking if we have a tx_buf?
>
> Yeah, as mentioned this RFC is not ready for merging - checkpatch.pl
> will complain about lines too long, and TODOs are sprinkled all over or
> not even mentioned. It's a Proof of Concept that a net_device could work
> for a wide range of spi and serdev based drivers, and on top this device
> has more than one channel, which may influence network-level design
> discussions.
>
> That said, I'll happily drop the second check. Thanks for spotting!
>
> >> + if (ret) {
> >> + dev_err(&spi->dev, "SPI radio address write
> failed\n");
> >> + return ret;
> >> + }
> >> +
> >> + ret = sx1301_write(ssx->parent, ssx->regs +
> REG_RADIO_X_DATA, (tx_buf && xfr->len >= 2) ? tx_buf[1] : 0);
> >> + if (ret) {
> >> + dev_err(&spi->dev, "SPI radio data write failed\n");
> >> + return ret;
> >> + }
> >
> > This looks awfully like you're coming in at the wrong abstraction layer
> > and the hardware actually implements a register abstraction rather than
> > a SPI one so you should be using regmap as the abstraction.
>
> I don't understand. Ben has suggested using regmap for the SPI _device_
> that we're talking to, which may be a good idea. But this SX1301 device
> in turn has two SPI _masters_ talking to an SX125x slave each. I don't
> see how using regmap instead of my wrappers avoids this spi_controller?
> The whole point of this spi_controller is to abstract and separate the
> SX1255 vs. SX1257 vs. whatever-radio-attached into a separate driver,
> instead of mixing it into the SX1301 driver - to me that looks cleaner
> and more extensible. It also has the side-effect that we could configure
> the two radios via DT (frequencies, clk output, etc.).
You want an SPI controller in the SX1301 as the down stream radios are SPI and could be attached directly to a host SPI bus, makes sense to have one radio driver and talk through the SX1301.
But you should use the regmap to access the SX1301 master controller registers.
Example I use with one SPI master and some clock info:
eg:
sx1301: sx1301@0 {
compatible = "semtech,sx1301";
reg = <0>;
#address-cells = <1>;
#size-cells = <0>;
spi-max-frequency = <8000000>;
gpios-reset = <&pioA 26 GPIO_ACTIVE_HIGH>;
clocks = <&radio1 0>, <&clkhs 0>;
clock-names = "clk32m", "clkhs";
radio0: sx1257@0 {
compatible = "semtech,sx125x";
reg = <0>;
spi-max-frequency = <8000000>;
tx;
clocks = <&tcxo 0>;
clock-names = "tcxo";
};
radio1: sx1257@1 {
compatible = "semtech,sx125x";
reg = <1>;
spi-max-frequency = <8000000>;
#clock-cells = <0>;
clocks = <&tcxo 0>;
clock-names = "tcxo";
clock-output-names = "clk32m";
};
};
> You will find a datasheet with some diagrams mentioning "SPI" at:
> https://www.semtech.com/products/wireless-rf/lora-gateways/SX1301
>
> >> + if (rx_buf) {
> >> + ret = sx1301_read(ssx->parent, ssx->regs +
> REG_RADIO_X_DATA_READBACK, &rx_buf[xfr->len - 1]);
> >> + if (ret) {
> >> + dev_err(&spi->dev, "SPI radio data read failed\n");
> >> + return ret;
> >> + }
> >> + }
> >
> > For a read we never set an address?
>
> To read, you first write the address via tx_buf, then either in the same
> transfer in the third byte or in a subsequent one-byte transfer as first
> byte you get the data.
>
> If you have better ideas how to structure this, do let me know.
>
> >> +static void sx1301_radio_setup(struct spi_controller *ctrl)
> >> +{
> >> + ctrl->mode_bits = SPI_CS_HIGH | SPI_NO_CS;
> >
> > This controller has no chip select but we provided a set_cs operation?
>
> Oops, I played around with those two options and was hoping SPI_NO_CS
> would avoid the undesired set_cs invocations, but it didn't work as
> expected and so I added the "if (enabled)" check above.
>
> Thanks for your review,
>
> Andreas
>
> --
> SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Felix Imendörffer, Jane Smithard, Graham Norton
> HRB 21284 (AG Nürnberg)
^ permalink raw reply
* Re: [PATCH] 6lowpan: iphc: reset mac_header after decompress to fix panic
From: Alexander Aring @ 2018-07-02 20:43 UTC (permalink / raw)
To: Michael Scott
Cc: Jukka Rissanen, David S. Miller, linux-bluetooth, linux-wpan,
netdev, linux-kernel
In-Reply-To: <516fbc65-cc4b-8016-de5a-e2240b779d15@opensourcefoundries.com>
Hi,
On Mon, Jul 02, 2018 at 12:45:41PM -0700, Michael Scott wrote:
> Hello Alexander,
>
...
> > Question is for me: which upper layer wants access MAC header here on
> > receive path?
> > It cannot parsed anyhow because so far I know no upper layer can parse
> > at the moment 802.15.4 frames (which is a complex format). Maybe over
> > some header_ops callback?
>
> I was testing a C program which performs NAT64 handling on packets
> destined to a certain IPv6 subnet (64:ff9b::). To do this, the application
> opens a RAW socket like this: sniff_sock = socket(PF_PACKET, SOCK_RAW,
> htons(ETH_P_ALL)); It then sets promiscuous mode and enters a looping call
> of:
> length = recv(sniff_sock, buffer, PACKET_BUFFER, MSG_TRUNC); My host PC
> kernel would then promptly crash on me. (I'm going to purposely avoid the
> obvious point of: this probably isn't the best way to parse packets for
> NAT64 translation as you will get every single packet incoming or outgoing
> on the host.) Turns out, testing the program on an 802.15.4 6lowpan
> interface exposed some of the issues which this mailing list (but not
> myself) is well aware of (no L2 data in the RAW packets) and also led me to
> debugging this patch to stop the kernel crash. TL;DR: To summarize, any
> PF_PACKET SOCK_RAW socket which recv()'s IPv6 data from a 6lowpan node will
> cause this kernel crash eventually (checked on kernel 4.15, 4.16, 4.17 and
> 4.18-rc1). - Mike
> >
"any PF_PACKET SOCK_RAW" can't be otherwise I would also see it with my
sniffer programs e.g. wireshark or tcpdump which use libpcap.
There need to be some different in the handling. This is what I have
currently in my mind.
I currently not sure how to set skb->mac_header if interface is RAW_IP.
It seems there is an indicator that mac header is not set. Example:
diff --git a/net/6lowpan/iphc.c b/net/6lowpan/iphc.c
index 6b1042e21656..e6ec2df3afe0 100644
--- a/net/6lowpan/iphc.c
+++ b/net/6lowpan/iphc.c
@@ -770,6 +770,7 @@ int lowpan_header_decompress(struct sk_buff *skb, const struct net_device *dev,
hdr.hop_limit, &hdr.daddr);
skb_push(skb, sizeof(hdr));
+ skb->mac_header = (typeof(skb->mac_header))~0U;
skb_reset_network_header(skb);
skb_copy_to_linear_data(skb, &hdr, sizeof(hdr));
Maybe we should lookup what skb->mac_header points to on tun interfaces
then do the same.
- Alex
^ permalink raw reply related
* Re: Regression introduced by "r8169: simplify rtl_set_mac_address"
From: Heiner Kallweit @ 2018-07-02 20:35 UTC (permalink / raw)
To: Corinna Vinschen; +Cc: netdev@vger.kernel.org
In-Reply-To: <20180702194812.GG3111@calimero.vinschen.de>
On 02.07.2018 21:48, Corinna Vinschen wrote:
> Hi,
>
> the patch 1f7aa2bc268e, "r8169: simplify rtl_set_mac_address",
> introduced a regression found by trying to team a r8169 NIC.
>
Thanks for reporting!
> Try the following (assuming the r8169 NIC is eth0):
>
> $ nmcli con add type team con-name team0 ifname nm-team config \
> '{"runner": {"name": "lacp"}, "link_watch": {"name": "ethtool"}}' \
> ipv4.method disable ipv6.method ignore
> $ nmcli con add type ethernet ifname eth0 con-name team0.0 master nm-team
> $ nmcli con up team0.0
> $ teamdctl nm-team port present eth0
> command call failed (No such device)
>
> Bisecting turned up commit 1f7aa2bc268e, "r8169: simplify
> rtl_set_mac_address" as the culprit. Reverting this patch fixes
> the issue and the teamdctl call succeeds.
>
> The reason is apparently the usage of eth_mac_addr here. eth_mac_addr
> calls eth_prepare_mac_addr_change which checks for IFF_LIVE_ADDR_CHANGE.
> Debugging shows this flag not being set on r8169, thus
> eth_prepare_mac_addr_change returns -EBUSY (no idea why userspace claims
> "No such device", rather than "Device or resource busy", but that's not
> the point here).
>
> Note that other devices like igb, don't call eth_mac_addr either, but
> rather call memcpy by themselves to copy the new MAC, just as the
> original r8169 code did, too. Consequentially this problem is not
> present on igb.
>
Doing the memcpy directly in the driver (like it was before) would be
a solution, however I'd consider this more a workaround because purpose
of the core functions is to encapsulate such details.
> I suggest to revert this change in the first place, but I wonder if
> we're not just missing to set IFF_LIVE_ADDR_CHANGE in a lot of drivers.
>
I'd prefer to keep the code and set flag IFF_LIVE_ADDR_CHANGE in the
driver. Setting this flag via ethtool --set-priv-flags in theory would
also be an option, however the r8169 driver doesn't implement the
ethtool_ops set_priv_flags callback.
>
> Thanks,
> Corinna
>
Rgds, Heiner
^ 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