* [PATCH] rhashtable: Fix missing elements when inserting.
From: Taehee Yoo @ 2017-05-24 1:49 UTC (permalink / raw)
To: davem; +Cc: netdev, Taehee Yoo
rhltable_insert_key() inserts a node into list of element,
if node's key is duplicated, so that it becomes the chain of
element(as known as rhead). Also bucket table points that element directly.
If a inserted node's element chain is located at third,
rhltable misses first and second element chain.
This issue is causion of to failture the rhltable_remove().
After this patch, rhltable_insert_key() inserts a node into second of
element's list, so that rhlist do not misses elements.
Signed-off-by: Taehee Yoo <ap420073@gmail.com>
---
include/linux/rhashtable.h | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/include/linux/rhashtable.h b/include/linux/rhashtable.h
index 7d56a7e..d3c24b9 100644
--- a/include/linux/rhashtable.h
+++ b/include/linux/rhashtable.h
@@ -762,11 +762,9 @@ static inline void *__rhashtable_insert_fast(
list = container_of(obj, struct rhlist_head, rhead);
plist = container_of(head, struct rhlist_head, rhead);
- RCU_INIT_POINTER(list->next, plist);
- head = rht_dereference_bucket(head->next, tbl, hash);
- RCU_INIT_POINTER(list->rhead.next, head);
- rcu_assign_pointer(*pprev, obj);
-
+ RCU_INIT_POINTER(list->next, rht_dereference_bucket(plist->next,
+ tbl, hash));
+ RCU_INIT_POINTER(plist->next, list);
goto good;
}
--
2.9.3
^ permalink raw reply related
* Re: [PATCH V3 net 1/3] vlan: Fix tcp checksum offloads in Q-in-Q vlans
From: Toshiaki Makita @ 2017-05-24 0:48 UTC (permalink / raw)
To: Vladislav Yasevich, netdev
Cc: alexander.duyck, Vladislav Yasevich, Michal Kubecek
In-Reply-To: <1495561123-1819-2-git-send-email-vyasevic@redhat.com>
On 2017/05/24 2:38, Vladislav Yasevich wrote:
> It appears that TCP checksum offloading has been broken for
> Q-in-Q vlans. The behavior was execerbated by the
> series
> commit afb0bc972b52 ("Merge branch 'stacked_vlan_tso'")
> that that enabled accleleration features on stacked vlans.
>
> However, event without that series, it is possible to trigger
> this issue. It just requires a lot more specialized configuration.
>
> The root cause is the interaction between how
> netdev_intersect_features() works, the features actually set on
> the vlan devices and HW having the ability to run checksum with
> longer headers.
>
> The issue starts when netdev_interesect_features() replaces
> NETIF_F_HW_CSUM with a combination of NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM,
> if the HW advertises IP|IPV6 specific checksums. This happens
> for tagged and multi-tagged packets. However, HW that enables
> IP|IPV6 checksum offloading doesn't gurantee that packets with
> arbitrarily long headers can be checksummed.
>
> This patch disables IP|IPV6 checksums on the packet for multi-tagged
> packets.
>
> CC: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
> CC: Michal Kubecek <mkubecek@suse.cz>
> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
> ---
Thank you for fixing it.
Acked-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
Toshiaki Makita
^ permalink raw reply
* [PATCH] net: jme: Remove unused functions
From: Matthias Kaehlcke @ 2017-05-24 0:27 UTC (permalink / raw)
To: Guo-Fu Tseng; +Cc: netdev, linux-kernel, Douglas Anderson, Matthias Kaehlcke
The functions jme_restart_tx_engine(), jme_pause_rx() and
jme_resume_rx() are not used. Removing them fixes the following warnings
when building with clang:
drivers/net/ethernet/jme.c:694:1: error: unused function
'jme_restart_tx_engine' [-Werror,-Wunused-function]
drivers/net/ethernet/jme.c:2393:20: error: unused function
'jme_pause_rx' [-Werror,-Wunused-function]
drivers/net/ethernet/jme.c:2406:20: error: unused function
'jme_resume_rx' [-Werror,-Wunused-function]
Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
drivers/net/ethernet/jme.c | 42 ------------------------------------------
1 file changed, 42 deletions(-)
diff --git a/drivers/net/ethernet/jme.c b/drivers/net/ethernet/jme.c
index f580b49e6b67..0e5083a48937 100644
--- a/drivers/net/ethernet/jme.c
+++ b/drivers/net/ethernet/jme.c
@@ -691,17 +691,6 @@ jme_enable_tx_engine(struct jme_adapter *jme)
}
static inline void
-jme_restart_tx_engine(struct jme_adapter *jme)
-{
- /*
- * Restart TX Engine
- */
- jwrite32(jme, JME_TXCS, jme->reg_txcs |
- TXCS_SELECT_QUEUE0 |
- TXCS_ENABLE);
-}
-
-static inline void
jme_disable_tx_engine(struct jme_adapter *jme)
{
int i;
@@ -2382,37 +2371,6 @@ jme_tx_timeout(struct net_device *netdev)
jme_reset_link(jme);
}
-static inline void jme_pause_rx(struct jme_adapter *jme)
-{
- atomic_dec(&jme->link_changing);
-
- jme_set_rx_pcc(jme, PCC_OFF);
- if (test_bit(JME_FLAG_POLL, &jme->flags)) {
- JME_NAPI_DISABLE(jme);
- } else {
- tasklet_disable(&jme->rxclean_task);
- tasklet_disable(&jme->rxempty_task);
- }
-}
-
-static inline void jme_resume_rx(struct jme_adapter *jme)
-{
- struct dynpcc_info *dpi = &(jme->dpi);
-
- if (test_bit(JME_FLAG_POLL, &jme->flags)) {
- JME_NAPI_ENABLE(jme);
- } else {
- tasklet_enable(&jme->rxclean_task);
- tasklet_enable(&jme->rxempty_task);
- }
- dpi->cur = PCC_P1;
- dpi->attempt = PCC_P1;
- dpi->cnt = 0;
- jme_set_rx_pcc(jme, PCC_P1);
-
- atomic_inc(&jme->link_changing);
-}
-
static void
jme_get_drvinfo(struct net_device *netdev,
struct ethtool_drvinfo *info)
--
2.13.0.219.gdb65acc882-goog
^ permalink raw reply related
* (unknown),
From: bcohen @ 2017-05-24 0:12 UTC (permalink / raw)
To: netdev
[-- Attachment #1: 07974248344583.zip --]
[-- Type: application/zip, Size: 3196 bytes --]
^ permalink raw reply
* pull request: bluetooth-next 2017-05-23
From: Johan Hedberg @ 2017-05-24 0:11 UTC (permalink / raw)
To: davem; +Cc: linux-bluetooth, netdev
[-- Attachment #1: Type: text/plain, Size: 3138 bytes --]
Hi Dave,
Here's the first Bluetooth & 802.15.4 pull request targeting the 4.13
kernel release.
- Bluetooth 5.0 improvements (Data Length Extensions and alternate PHY)
- Support for new Intel Bluetooth adapter [[8087:0aaa]
- Various fixes to ieee802154 code
- Various fixes to HCI UART code
Please let me know if there are any issues pulling. Thanks.
Johan
---
The following changes since commit 1fc4d180b3c6bed0e7f5160bcd553aec89594962:
Merge branch 'phy-marvell-cleanups' (2017-05-17 16:27:52 -0400)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git for-upstream
for you to fetch changes up to 7dab5467647be42736dcabcd5d035c7b571f4653:
net: ieee802154: fix potential null pointer dereference (2017-05-23 20:12:53 +0200)
----------------------------------------------------------------
Alexander Aring (1):
MAINTAINERS: update my mail address
Dean Jenkins (1):
Bluetooth: hci_ldisc: Use rwlocking to avoid closing proto races
Guodong Xu (1):
Bluetooth: hci_ll: Fix download_firmware() return when __hci_cmd_sync fails
Gustavo A. R. Silva (1):
net: ieee802154: fix potential null pointer dereference
Jürg Billeter (1):
Bluetooth: btintel: Add MODULE_FIRMWARE entries for iBT 3.5 controllers
Lin Zhang (2):
net: ieee802154: remove explicit set skb->sk
net: ieee802154: fix net_device reference release too early
Loic Poulain (1):
Bluetooth: btwilink: Fix unexpected skb free
Marcel Holtmann (5):
Bluetooth: Set LE Suggested Default Data Length to maximum
Bluetooth: Enable LE Channel Selection Algorithm event
Bluetooth: Enable LE PHY Update Complete event
Bluetooth: Set LE Default PHY preferences
Bluetooth: Skip vendor diagnostic configuration for HCI User Channel
Markus Elfring (3):
Bluetooth: Delete error messages for failed memory allocations in two functions
ieee802154: ca8210: Delete an error message for a failed memory allocation in ca8210_probe()
ieee802154: ca8210: Delete an error message for a failed memory allocation in ca8210_skb_rx()
Tedd Ho-Jeong An (1):
Bluetooth: Add support for Intel Bluetooth device 9460/9560 [8087:0aaa]
Tobias Regnery (2):
Bluetooth: hci_uart: fix kconfig dependency
Bluetooth: hci_nokia: select BT_HCIUART_H4
MAINTAINERS | 4 ++--
drivers/bluetooth/Kconfig | 3 ++-
drivers/bluetooth/btintel.c | 2 ++
drivers/bluetooth/btusb.c | 4 ++++
drivers/bluetooth/btwilink.c | 1 -
drivers/bluetooth/hci_ldisc.c | 40 ++++++++++++++++++++++++++++++-----
drivers/bluetooth/hci_ll.c | 1 +
drivers/bluetooth/hci_uart.h | 1 +
drivers/net/ieee802154/ca8210.c | 12 ++++-------
include/net/bluetooth/hci.h | 8 +++++++
net/bluetooth/ecdh_helper.c | 11 +++-------
net/bluetooth/hci_core.c | 46 ++++++++++++++++++++++++++++++++++++-----
net/ieee802154/socket.c | 10 ++++-----
13 files changed, 107 insertions(+), 36 deletions(-)
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: Alignment in BPF verifier
From: Alexei Starovoitov @ 2017-05-23 23:59 UTC (permalink / raw)
To: Edward Cree, Daniel Borkmann, David Miller
Cc: alexei.starovoitov, netdev, Josef Bacik
In-Reply-To: <748ff318-84dc-d338-67bd-2fd62ebcf3b8@solarflare.com>
On 5/23/17 10:43 AM, Edward Cree wrote:
> Another issue: it looks like the min/max_value handling for subtraction is
> bogus. In adjust_reg_min_max_vals() we have
> if (dst_reg->min_value != BPF_REGISTER_MIN_RANGE)
> dst_reg->min_value -= min_val;
> if (dst_reg->max_value != BPF_REGISTER_MAX_RANGE)
> dst_reg->max_value -= max_val;
> where min_val and max_val refer to the src_reg.
> But surely they should be used the other way round; if (say) 2 <= R1 <= 6
> and 1 <= R2 <= 4, then this will claim 1 <= (R1 - R2) <= 2, whereas really
> (R1 - R2) could be anything from -2 to 5.
> This also means that the code just above the switch,
> if (min_val == BPF_REGISTER_MIN_RANGE)
> dst_reg->min_value = BPF_REGISTER_MIN_RANGE;
> if (max_val == BPF_REGISTER_MAX_RANGE)
> dst_reg->max_value = BPF_REGISTER_MAX_RANGE;
> is wrong, since e.g. subtracting MAX_RANGE needs to blow our min_value,
> not our max_value.
right. good catch. I have a feeling we discussed similar thing before.
May be some patch felt through the cracks.
That's the reason the fancy verifier analysis is root only.
I'm assuming you're going to send a fix?
Thanks!
^ permalink raw reply
* [patch iproute2] ipvtap: Adding support for ipvtap device management
From: Sainath Grandhi @ 2017-05-23 23:22 UTC (permalink / raw)
To: netdev; +Cc: stephen, Sainath Grandhi
This patch adds support for managing ipvtap devices using ip link. ipvtap support
is added to linux with commit 235a9d89da976e2975b3de9afc0bed7b72557983
---
ip/iplink_ipvlan.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/ip/iplink_ipvlan.c b/ip/iplink_ipvlan.c
index f7735f3..153aa2f 100644
--- a/ip/iplink_ipvlan.c
+++ b/ip/iplink_ipvlan.c
@@ -1,4 +1,4 @@
-/* iplink_ipvlan.c IPVLAN device support
+/* iplink_ipvlan.c IPVLAN/IPVTAP device support
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License
@@ -90,3 +90,11 @@ struct link_util ipvlan_link_util = {
.print_opt = ipvlan_print_opt,
.print_help = ipvlan_print_help,
};
+
+struct link_util ipvtap_link_util = {
+ .id = "ipvtap",
+ .maxattr = IFLA_IPVLAN_MAX,
+ .parse_opt = ipvlan_parse_opt,
+ .print_opt = ipvlan_print_opt,
+ .print_help = ipvlan_print_help,
+};
--
2.7.4
^ permalink raw reply related
* [PATCH] net: fix potential null pointer dereference
From: Gustavo A. R. Silva @ 2017-05-23 23:18 UTC (permalink / raw)
To: Pablo Neira Ayuso, Harald Welte
Cc: osmocom-net-gprs, netdev, linux-kernel, Gustavo A. R. Silva
Add null check to avoid a potential null pointer dereference.
Addresses-Coverity-ID: 1408831
Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
---
drivers/net/gtp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index 4fea1b3..7b652bb 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -873,7 +873,7 @@ static struct gtp_dev *gtp_find_dev(struct net *src_net, struct nlattr *nla[])
/* Check if there's an existing gtpX device to configure */
dev = dev_get_by_index_rcu(net, nla_get_u32(nla[GTPA_LINK]));
- if (dev->netdev_ops == >p_netdev_ops)
+ if (dev && dev->netdev_ops == >p_netdev_ops)
gtp = netdev_priv(dev);
put_net(net);
--
2.5.0
^ permalink raw reply related
* [PATCH net-next] geneve: fix fill_info when using collect_metadata
From: Eric Garver @ 2017-05-23 22:37 UTC (permalink / raw)
To: netdev; +Cc: pravin shelar
Since 9b4437a5b870 ("geneve: Unify LWT and netdev handling.") fill_info
does not return UDP_ZERO_CSUM6_RX when using COLLECT_METADATA. This is
because it uses ip_tunnel_info_af() with the device level info, which is
not valid for COLLECT_METADATA.
Fix by checking for the presence of the actual sockets.
Fixes: 9b4437a5b870 ("geneve: Unify LWT and netdev handling.")
Signed-off-by: Eric Garver <e@erig.me>
---
drivers/net/geneve.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
index dec5d563ab19..959fd12d2e67 100644
--- a/drivers/net/geneve.c
+++ b/drivers/net/geneve.c
@@ -1293,7 +1293,7 @@ static int geneve_fill_info(struct sk_buff *skb, const struct net_device *dev)
if (nla_put_u32(skb, IFLA_GENEVE_ID, vni))
goto nla_put_failure;
- if (ip_tunnel_info_af(info) == AF_INET) {
+ if (rtnl_dereference(geneve->sock4)) {
if (nla_put_in_addr(skb, IFLA_GENEVE_REMOTE,
info->key.u.ipv4.dst))
goto nla_put_failure;
@@ -1302,8 +1302,10 @@ static int geneve_fill_info(struct sk_buff *skb, const struct net_device *dev)
!!(info->key.tun_flags & TUNNEL_CSUM)))
goto nla_put_failure;
+ }
+
#if IS_ENABLED(CONFIG_IPV6)
- } else {
+ if (rtnl_dereference(geneve->sock6)) {
if (nla_put_in6_addr(skb, IFLA_GENEVE_REMOTE6,
&info->key.u.ipv6.dst))
goto nla_put_failure;
@@ -1315,8 +1317,8 @@ static int geneve_fill_info(struct sk_buff *skb, const struct net_device *dev)
if (nla_put_u8(skb, IFLA_GENEVE_UDP_ZERO_CSUM6_RX,
!geneve->use_udp6_rx_checksums))
goto nla_put_failure;
-#endif
}
+#endif
if (nla_put_u8(skb, IFLA_GENEVE_TTL, info->key.ttl) ||
nla_put_u8(skb, IFLA_GENEVE_TOS, info->key.tos) ||
--
2.12.0
^ permalink raw reply related
* Re: [PATCH] net: phy: put genphy_config_init's EXPORT_SYMBOL directly after the function
From: Florian Fainelli @ 2017-05-23 22:28 UTC (permalink / raw)
To: Uwe Kleine-König, Andrew Lunn, David S. Miller; +Cc: netdev, kernel
In-Reply-To: <20170523222607.13645-1-u.kleine-koenig@pengutronix.de>
On 05/23/2017 03:26 PM, Uwe Kleine-König wrote:
> Commit af6b6967d6e1 ("net: phy: export genphy_config_init()") introduced
> this EXPORT_SYMBOL and put it after gen10g_soft_reset() instead of
> directly after genphy_config_init. Probably this happend when the patch
> was applied because http://patchwork.ozlabs.org/patch/339622/ looks ok.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Acked-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* [PATCH] net: phy: put genphy_config_init's EXPORT_SYMBOL directly after the function
From: Uwe Kleine-König @ 2017-05-23 22:26 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, David S. Miller; +Cc: netdev, kernel
Commit af6b6967d6e1 ("net: phy: export genphy_config_init()") introduced
this EXPORT_SYMBOL and put it after gen10g_soft_reset() instead of
directly after genphy_config_init. Probably this happend when the patch
was applied because http://patchwork.ozlabs.org/patch/339622/ looks ok.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/net/phy/phy_device.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 1219eeab69d1..0780e9f9e167 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1571,13 +1571,13 @@ int genphy_config_init(struct phy_device *phydev)
return 0;
}
+EXPORT_SYMBOL(genphy_config_init);
static int gen10g_soft_reset(struct phy_device *phydev)
{
/* Do nothing for now */
return 0;
}
-EXPORT_SYMBOL(genphy_config_init);
static int gen10g_config_init(struct phy_device *phydev)
{
--
2.11.0
^ permalink raw reply related
* [PATCH net-next] tcp: better validation of received ack sequences
From: Eric Dumazet @ 2017-05-23 22:24 UTC (permalink / raw)
To: Paul Fiterau Brostean, David Miller
Cc: netdev, Frits Vaandrager, Neal Cardwell, Yuchung Cheng,
Soheil Hassas Yeganeh
In-Reply-To: <fbb92f49-5eca-f411-dcab-253d2a222623@science.ru.nl>
From: Eric Dumazet <edumazet@google.com>
Paul Fiterau Brostean reported :
<quote>
Linux TCP stack we analyze exhibits behavior that seems odd to me.
The scenario is as follows (all packets have empty payloads, no window
scaling, rcv/snd window size should not be a factor):
TEST HARNESS (CLIENT) LINUX SERVER
1. - LISTEN (server listen,
then accepts)
2. - --> <SEQ=100><CTL=SYN> --> SYN-RECEIVED
3. - <-- <SEQ=300><ACK=101><CTL=SYN,ACK> <-- SYN-RECEIVED
4. - --> <SEQ=101><ACK=301><CTL=ACK> --> ESTABLISHED
5. - <-- <SEQ=301><ACK=101><CTL=FIN,ACK> <-- FIN WAIT-1 (server
opts to close the data connection calling "close" on the connection
socket)
6. - --> <SEQ=101><ACK=99999><CTL=FIN,ACK> --> CLOSING (client sends
FIN,ACK with not yet sent acknowledgement number)
7. - <-- <SEQ=302><ACK=102><CTL=ACK> <-- CLOSING (ACK is 102
instead of 101, why?)
... (silence from CLIENT)
8. - <-- <SEQ=301><ACK=102><CTL=FIN,ACK> <-- CLOSING
(retransmission, again ACK is 102)
Now, note that packet 6 while having the expected sequence number,
acknowledges something that wasn't sent by the server. So I would
expect
the packet to maybe prompt an ACK response from the server, and then be
ignored. Yet it is not ignored and actually leads to an increase of the
acknowledgement number in the server's retransmission of the FIN,ACK
packet. The explanation I found is that the FIN in packet 6 was
processed, despite the acknowledgement number being unacceptable.
Further experiments indeed show that the server processes this FIN,
transitioning to CLOSING, then on receiving an ACK for the FIN it had
send in packet 5, the server (or better said connection) transitions
from CLOSING to TIME_WAIT (as signaled by netstat).
</quote>
Indeed, tcp_rcv_state_process() calls tcp_ack() but
does not exploit the @acceptable status but for TCP_SYN_RECV
state.
What we want here is to send a challenge ACK, if not in TCP_SYN_RECV
state. TCP_FIN_WAIT1 state is not the only state we should fix.
Add a FLAG_NO_CHALLENGE_ACK so that tcp_rcv_state_process()
can choose to send a challenge ACK and discard the packet instead
of wrongly change socket state.
With help from Neal Cardwell.
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: Paul Fiterau Brostean <p.fiterau-brostean@science.ru.nl>
Cc: Neal Cardwell <ncardwell@google.com>
Cc: Yuchung Cheng <ycheng@google.com>
Cc: Soheil Hassas Yeganeh <soheil@google.com>
---
Note for Googlers : Google-Bug-Id: 37204158
net/ipv4/tcp_input.c | 24 +++++++++++-------------
1 file changed, 11 insertions(+), 13 deletions(-)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 2fa55f57ac06584bfd9b555799ceb3bbfb7e1b4e..c3bdcbcf544793ba410c618130586bf7d3963da6 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -112,6 +112,7 @@ int sysctl_tcp_invalid_ratelimit __read_mostly = HZ/2;
#define FLAG_DSACKING_ACK 0x800 /* SACK blocks contained D-SACK info */
#define FLAG_SACK_RENEGING 0x2000 /* snd_una advanced to a sacked seq */
#define FLAG_UPDATE_TS_RECENT 0x4000 /* tcp_replace_ts_recent() */
+#define FLAG_NO_CHALLENGE_ACK 0x8000 /* do not call tcp_send_challenge_ack() */
#define FLAG_ACKED (FLAG_DATA_ACKED|FLAG_SYN_ACKED)
#define FLAG_NOT_DUP (FLAG_DATA|FLAG_WIN_UPDATE|FLAG_ACKED)
@@ -3568,7 +3569,8 @@ static int tcp_ack(struct sock *sk, const struct sk_buff *skb, int flag)
if (before(ack, prior_snd_una)) {
/* RFC 5961 5.2 [Blind Data Injection Attack].[Mitigation] */
if (before(ack, prior_snd_una - tp->max_window)) {
- tcp_send_challenge_ack(sk, skb);
+ if (!(flag & FLAG_NO_CHALLENGE_ACK))
+ tcp_send_challenge_ack(sk, skb);
return -1;
}
goto old_ack;
@@ -5951,13 +5953,17 @@ int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb)
/* step 5: check the ACK field */
acceptable = tcp_ack(sk, skb, FLAG_SLOWPATH |
- FLAG_UPDATE_TS_RECENT) > 0;
+ FLAG_UPDATE_TS_RECENT |
+ FLAG_NO_CHALLENGE_ACK) > 0;
+ if (!acceptable) {
+ if (sk->sk_state == TCP_SYN_RECV)
+ return 1; /* send one RST */
+ tcp_send_challenge_ack(sk, skb);
+ goto discard;
+ }
switch (sk->sk_state) {
case TCP_SYN_RECV:
- if (!acceptable)
- return 1;
-
if (!tp->srtt_us)
tcp_synack_rtt_meas(sk, req);
@@ -6026,14 +6032,6 @@ int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb)
* our SYNACK so stop the SYNACK timer.
*/
if (req) {
- /* Return RST if ack_seq is invalid.
- * Note that RFC793 only says to generate a
- * DUPACK for it but for TCP Fast Open it seems
- * better to treat this case like TCP_SYN_RECV
- * above.
- */
- if (!acceptable)
- return 1;
/* We no longer need the request sock. */
reqsk_fastopen_remove(sk, req, false);
tcp_rearm_rto(sk);
^ permalink raw reply related
* [patch iproute2] tc: flower: add support for tcp flags
From: Jiri Pirko @ 2017-05-23 21:51 UTC (permalink / raw)
To: netdev; +Cc: davem, idosch, yotamg, mlxsw, Yuval.Mintz
In-Reply-To: <20170523164048.16514-1-jiri@resnulli.us>
From: Jiri Pirko <jiri@mellanox.com>
Allow user to insert a flower classifier filter rule which includes
match for tcp flags.
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
v1->v2:
- removed forgotten debug printout
- fixed mask parsing as reported by Or
---
include/linux/pkt_cls.h | 3 +++
man/man8/tc-flower.8 | 8 +++++++
tc/f_flower.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 73 insertions(+)
diff --git a/include/linux/pkt_cls.h b/include/linux/pkt_cls.h
index d613be3..ce9dfb9 100644
--- a/include/linux/pkt_cls.h
+++ b/include/linux/pkt_cls.h
@@ -450,6 +450,9 @@ enum {
TCA_FLOWER_KEY_MPLS_TC, /* u8 - 3 bits */
TCA_FLOWER_KEY_MPLS_LABEL, /* be32 - 20 bits */
+ TCA_FLOWER_KEY_TCP_FLAGS, /* be16 */
+ TCA_FLOWER_KEY_TCP_FLAGS_MASK, /* be16 */
+
__TCA_FLOWER_MAX,
};
diff --git a/man/man8/tc-flower.8 b/man/man8/tc-flower.8
index ba29065..7648079 100644
--- a/man/man8/tc-flower.8
+++ b/man/man8/tc-flower.8
@@ -35,6 +35,8 @@ flower \- flow based traffic control filter
.IR PREFIX " | { "
.BR dst_port " | " src_port " } "
.IR port_number " } | "
+.B tcp_flags
+.IR MASKED_TCP_FLAGS " | "
.B type
.IR MASKED_TYPE " | "
.B code
@@ -136,6 +138,12 @@ Match on layer 4 protocol source or destination port number. Only available for
.BR ip_proto " values " udp ", " tcp " and " sctp
which have to be specified in beforehand.
.TP
+.BI tcp_flags " MASKED_TCP_FLAGS"
+Match on TCP flags represented as 12bit bitfield in in hexadecimal format.
+A mask may be optionally provided to limit the bits which are matched. A mask
+is provided by following the value with a slash and then the mask. If the mask
+is missing then a match on all bits is assumed.
+.TP
.BI type " MASKED_TYPE"
.TQ
.BI code " MASKED_CODE"
diff --git a/tc/f_flower.c b/tc/f_flower.c
index ebc63ca..1b6b46e 100644
--- a/tc/f_flower.c
+++ b/tc/f_flower.c
@@ -57,6 +57,7 @@ static void explain(void)
" src_ip PREFIX |\n"
" dst_port PORT-NUMBER |\n"
" src_port PORT-NUMBER |\n"
+ " tcp_flags MASKED-TCP_FLAGS |\n"
" type MASKED-ICMP-TYPE |\n"
" code MASKED-ICMP-CODE |\n"
" arp_tip IPV4-PREFIX |\n"
@@ -474,6 +475,41 @@ static int flower_parse_port(char *str, __u8 ip_proto,
return 0;
}
+#define TCP_FLAGS_MAX_MASK 0xfff
+
+static int flower_parse_tcp_flags(char *str, int flags_type, int mask_type,
+ struct nlmsghdr *n)
+{
+ char *slash;
+ int ret, err = -1;
+ __u16 flags;
+
+ slash = strchr(str, '/');
+ if (slash)
+ *slash = '\0';
+
+ ret = get_u16(&flags, str, 16);
+ if (ret < 0 || flags & ~TCP_FLAGS_MAX_MASK)
+ goto err;
+
+ addattr16(n, MAX_MSG, flags_type, htons(flags));
+
+ if (slash) {
+ ret = get_u16(&flags, slash + 1, 16);
+ if (ret < 0 || flags & ~TCP_FLAGS_MAX_MASK)
+ goto err;
+ } else {
+ flags = TCP_FLAGS_MAX_MASK;
+ }
+ addattr16(n, MAX_MSG, mask_type, htons(flags));
+
+ err = 0;
+err:
+ if (slash)
+ *slash = '/';
+ return err;
+}
+
static int flower_parse_key_id(const char *str, int type, struct nlmsghdr *n)
{
int ret;
@@ -671,6 +707,16 @@ static int flower_parse_opt(struct filter_util *qu, char *handle,
fprintf(stderr, "Illegal \"src_port\"\n");
return -1;
}
+ } else if (matches(*argv, "tcp_flags") == 0) {
+ NEXT_ARG();
+ ret = flower_parse_tcp_flags(*argv,
+ TCA_FLOWER_KEY_TCP_FLAGS,
+ TCA_FLOWER_KEY_TCP_FLAGS_MASK,
+ n);
+ if (ret < 0) {
+ fprintf(stderr, "Illegal \"tcp_flags\"\n");
+ return -1;
+ }
} else if (matches(*argv, "type") == 0) {
NEXT_ARG();
ret = flower_parse_icmp(*argv, eth_type, ip_proto,
@@ -1000,6 +1046,19 @@ static void flower_print_port(FILE *f, char *name, struct rtattr *attr)
fprintf(f, "\n %s %d", name, rta_getattr_be16(attr));
}
+static void flower_print_tcp_flags(FILE *f, char *name,
+ struct rtattr *flags_attr,
+ struct rtattr *mask_attr)
+{
+ if (!flags_attr)
+ return;
+ fprintf(f, "\n %s %x", name, rta_getattr_be16(flags_attr));
+ if (!mask_attr)
+ return;
+ fprintf(f, "/%x", rta_getattr_be16(mask_attr));
+}
+
+
static void flower_print_key_id(FILE *f, const char *name,
struct rtattr *attr)
{
@@ -1110,6 +1169,9 @@ static int flower_print_opt(struct filter_util *qu, FILE *f,
if (nl_type >= 0)
flower_print_port(f, "src_port", tb[nl_type]);
+ flower_print_tcp_flags(f, "tcp_flags", tb[TCA_FLOWER_KEY_TCP_FLAGS],
+ tb[TCA_FLOWER_KEY_TCP_FLAGS_MASK]);
+
nl_type = flower_icmp_attr_type(eth_type, ip_proto,
FLOWER_ICMP_FIELD_TYPE);
nl_mask_type = flower_icmp_attr_mask_type(eth_type, ip_proto,
--
2.9.3
^ permalink raw reply related
* Re: bond link state mismatch, rtnl_trylock() vs rtnl_lock()
From: Nithin Sujir @ 2017-05-23 21:35 UTC (permalink / raw)
To: Mahesh Bandewar (महेश बंडेवार)
Cc: linux-netdev
In-Reply-To: <CAF2d9jiUh2iVG4=igP0+e6dzZkff_h8u02taQts6hC2B2qsM2g@mail.gmail.com>
On 5/23/2017 2:30 PM, Mahesh Bandewar (महेश बंडेवार) wrote:
> On Tue, May 23, 2017 at 12:32 PM, Nithin Sujir <nsujir@tintri.com> wrote:
>> diff --git a/drivers/net/bonding/bond_main.c
>> b/drivers/net/bonding/bond_main.c
>> index 5dca77e..1f60503 100644
>> --- a/drivers/net/bonding/bond_main.c
>> +++ b/drivers/net/bonding/bond_main.c
>> @@ -2614,8 +2614,7 @@ static void bond_loadbalance_arp_mon(struct
>> work_struct *work)
>> rcu_read_unlock();
>>
>> if (do_failover || slave_state_changed) {
>> - if (!rtnl_trylock())
>> - goto re_arm;
>> + rtnl_lock();
> Nitin, you can't do this. The tryRTNL code is to prevent deadlock
> during work-cancellation during bond_close().
Thanks, Mahesh. Yes, Jay pointed me to your patch and I will take a look
at how to use a similar approach.
Nithin.
>> if (slave_state_changed) {
>> bond_slave_state_change(bond);
>>
>>
^ permalink raw reply
* Re: bond link state mismatch, rtnl_trylock() vs rtnl_lock()
From: Mahesh Bandewar (महेश बंडेवार) @ 2017-05-23 21:30 UTC (permalink / raw)
To: Nithin Sujir; +Cc: linux-netdev
In-Reply-To: <24c0747b-df09-66da-f3ac-a393a5902a72@tintri.com>
On Tue, May 23, 2017 at 12:32 PM, Nithin Sujir <nsujir@tintri.com> wrote:
> Hi,
> We're encountering a problem in 4.4 LTS where, rarely, the bond link state
> is not updated when the slave link changes.
>
> I've traced the issue to the arp monitor unable to get the rtnl lock. The
> sequence resulting in failure is as below.
>
> bond_loadbalance_arp_mon() periodically called, if slave link is _down_, it
> checks if the slave is sending/receiving packets. If it is, it sets flags to
> be processed later down the function for bond link update. However, it sets
> the slave->link right away.
>
> if (slave->link != BOND_LINK_UP) {
> if (bond_time_in_interval(bond, trans_start, 1) &&
> bond_time_in_interval(bond, slave->last_rx, 1))
> {
>
> slave->link = BOND_LINK_UP;
> slave_state_changed = 1;
>
>
> Later down the function, it tries to get the rtnl_lock. If it doesn't get
> it, it rearms and returns.
>
> if (do_failover || slave_state_changed) {
> if (!rtnl_trylock())
> goto re_arm; <-- returns here
>
> if (slave_state_changed) {
> bond_slave_state_change(bond);
>
> This is the problem. The next time this function is called, the slave->link
> is already marked UP. And we will never update the bond link state to UP.
>
> Changing the rtnl_trylock() -> rtnl_lock() _does_ fix the issue.
>
> Is this the right way to fix it? If it is, I can submit this formally.
>
> What are the guidelines around using rtnl_lock() vs rtnl_trylock()? Some
> places are using rtnl_lock() and other rtnl_trylock(). Sorry, I couldn't
> find much via a google search or in Documentation/.
>
> Thanks,
> Nithin.
>
> --------------------
>
> diff --git a/drivers/net/bonding/bond_main.c
> b/drivers/net/bonding/bond_main.c
> index 5dca77e..1f60503 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -2614,8 +2614,7 @@ static void bond_loadbalance_arp_mon(struct
> work_struct *work)
> rcu_read_unlock();
>
> if (do_failover || slave_state_changed) {
> - if (!rtnl_trylock())
> - goto re_arm;
> + rtnl_lock();
Nitin, you can't do this. The tryRTNL code is to prevent deadlock
during work-cancellation during bond_close().
>
> if (slave_state_changed) {
> bond_slave_state_change(bond);
>
>
^ permalink raw reply
* Re: Alignment in BPF verifier
From: Daniel Borkmann @ 2017-05-23 21:27 UTC (permalink / raw)
To: Alexei Starovoitov, Edward Cree, David Miller; +Cc: alexei.starovoitov, netdev
In-Reply-To: <ad2162c3-1286-ab35-464a-8d21a1adc94f@fb.com>
On 05/23/2017 09:45 PM, Alexei Starovoitov wrote:
> On 5/23/17 7:41 AM, Edward Cree wrote:
>> I'm still plugging away at this... it's going to be quite a big patch and
>> rewrite a lot of stuff (and I'm not sure I'll be able to break it into
>> smaller bisectable patches).
>> And of course I have more questions. In check_packet_ptr_add(), we
>> forbid adding a negative constant to a packet ptr. Is there some
>> principled reason for that, or is it just because the bounds checking is
>> hard? It seems like, if imm + reg->off > 0 (suitably carefully checked
>> to avoid overflow etc.), then the subtraction should be legal. Indeed,
>> even if the reg->off (fixed part of offset) is zero, if the variable part
>> is known (min_value) to be >= -imm, the subtraction should be safe.
>
> adding negative imm to pkt_ptr is ok, but what is the use case?
> Do you see llvm generating such code?
Btw, currently, you kind of have it in a limited way via BPF_STX_MEM()
and BPF_LDX_MEM() when accessing pkt data through the offset, which
can be negative on the insn level.
> I think if we try to track everything with the current shape of
> state pruning, the verifier will stop accepting old programs
> because it reaches complexity limit.
>
> I think we need to rearchitect the whole thing.
> I was thinking of doing it compiler-style. Convert to ssa and
> do traditional data flow analysis, use-def chains, register liveness
> then pruning heuristics won't be necessary and verifier should be
> able to check everything in more or less single pass.
> Things like register liveness can be done without ssa. It can
> be used to augment existing pruning, since it will know which
> registers are dead, so they don't have to be compared, but it
> feels half-way. I'd rather go all the way.
>
>> On 20/05/17 00:05, Daniel Borkmann wrote:
>>> Besides PTR_TO_PACKET also PTR_TO_MAP_VALUE_OR_NULL uses it to
>>> track all registers (incl. spilled ones) with the same reg->id
>>> that originated from the same map lookup. After the reg type is
>>> then migrated to either PTR_TO_MAP_VALUE (resp. CONST_PTR_TO_MAP
>>> for map in map) or UNKNOWN_VALUE depending on the branch, the
>>> reg->id is then reset to 0 again. Whole reason for this is that
>>> LLVM generates code where it can move and/or spill a reg of type
>>> PTR_TO_MAP_VALUE_OR_NULL to other regs before we do the NULL
>>> test on it, and later on it expects that the spilled or moved
>>> regs work wrt access. So they're marked with an id and then all
>>> of them are type migrated. So here meaning of reg->id is different
>>> than in PTR_TO_PACKET case.
>> Hmm, that means that we can't do arithmetic on a
>> PTR_TO_MAP_VALUE_OR_NULL, we have to convert it to a PTR_TO_MAP_VALUE
>> first by NULL-checking it. That's probably fine, but I can just about
>> imagine some compiler optimisation reordering them. Any reason not to
>> split this out into a different reg->field, rather than overloading id?
>
> 'id' is sort of like 'version' of a pointer and has the same meaning in
> both cases. How exactly do you see this split?
Also, same id is never reused once generated and later propagated
through regs. So far we haven't run into this kind of optimization
from llvm side yet, but others which led to requiring the id marker
(see 57a09bf0a416). I could imagine it might be needed at some point,
though where we later transition directly to PTR_TO_MAP_VALUE_ADJ
after NULL check. Out of curiosity, did you run into it with llvm?
>> Of course that would need (more) caution wrt. states_equal(), but it
>> looks like I'll be mangling that a lot anyway - for instance, we don't
>> want to just use memcmp() to compare alignments, we want to check that
>> our alignment is stricter than the old alignment. (Of course memcmp()
>> is a conservative check, so the "memcmp() the whole reg_state" fast
>> path can remain.)
>
> yes. that would be good improvement. Not sure how much it will help
> the pruning though.
^ permalink raw reply
* [PATCH net-next 0/4] More marvell phy cleanups
From: Andrew Lunn @ 2017-05-23 21:24 UTC (permalink / raw)
To: David Miller; +Cc: Florian Fainelli, netdev, Andrew Lunn
This patchset continues the cleanup of the Marvell PHY driver. These
phys use pages to allow more than the 32 registers that fit into the
MDIO address space. Cleanup the code used for changing pages.
Andrew Lunn (4):
net: phy: marvell: #defines for copper and fibre pages
net: phy: marvell: More hidden page changes refactored
net: phy: marvell: helper to get and set page
net: phy: marvell: Uniform page names
drivers/net/phy/marvell.c | 181 +++++++++++++++++++++++++---------------------
1 file changed, 98 insertions(+), 83 deletions(-)
--
2.11.0
^ permalink raw reply
* Re: [PATCH net] netfilter: do not hold dev in ipt_CLUSTERIP
From: Pablo Neira Ayuso @ 2017-05-23 21:26 UTC (permalink / raw)
To: Xin Long; +Cc: network dev, netfilter-devel, davem, fw
In-Reply-To: <cc6ebc3f8784d8d537737a1e18b443a2e0c7313a.1495271286.git.lucien.xin@gmail.com>
On Sat, May 20, 2017 at 05:08:06PM +0800, Xin Long wrote:
> It's a terrible thing to hold dev in iptables target. When the dev is
> being removed, unregister_netdevice has to wait for the dev to become
> free. dmesg will keep logging the err:
>
> kernel:unregister_netdevice: waiting for veth0_in to become free. \
> Usage count = 1
>
> until iptables rules with this target are removed manually.
>
> The worse thing is when deleting a netns, a virtual nic will be deleted
> instead of reset to init_net in default_device_ops exit/exit_batch. As
> it is earlier than to flush the iptables rules in iptable_filter_net_ops
> exit, unregister_netdevice will block to wait for the nic to become free.
>
> As unregister_netdevice is actually waiting for iptables rules flushing
> while iptables rules have to be flushed after unregister_netdevice. This
> 'dead lock' will cause unregister_netdevice to block there forever. As
> the netns is not available to operate at that moment, iptables rules can
> not even be flushed manually either.
>
> The reproducer can be:
>
> # ip netns add test
> # ip link add veth0_in type veth peer name veth0_out
> # ip link set veth0_in netns test
> # ip netns exec test ip link set lo up
> # ip netns exec test ip link set veth0_in up
> # ip netns exec test iptables -I INPUT -d 1.2.3.4 -i veth0_in -j \
> CLUSTERIP --new --clustermac 89:d4:47:eb:9a:fa --total-nodes 3 \
> --local-node 1 --hashmode sourceip-sourceport
> # ip netns del test
>
> This issue can be triggered by all virtual nics with ipt_CLUSTERIP.
>
> This patch is to fix it by not holding dev in ipt_CLUSTERIP, but only
> save dev->ifindex instead of dev. When removing the mc from the dev,
> it will get dev by c->ifindex through dev_get_by_index.
>
> Note that it doesn't save dev->name but dev->ifindex, as a dev->name
> can be changed, it will confuse ipt_CLUSTERIP.
>
> Reported-by: Jianlin Shi <jishi@redhat.com>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
OK. Let's fix this finally... One comment below.
> net/ipv4/netfilter/ipt_CLUSTERIP.c | 31 ++++++++++++++++++-------------
> 1 file changed, 18 insertions(+), 13 deletions(-)
>
> diff --git a/net/ipv4/netfilter/ipt_CLUSTERIP.c b/net/ipv4/netfilter/ipt_CLUSTERIP.c
> index 038f293..d1adb2f 100644
> --- a/net/ipv4/netfilter/ipt_CLUSTERIP.c
> +++ b/net/ipv4/netfilter/ipt_CLUSTERIP.c
> @@ -47,7 +47,7 @@ struct clusterip_config {
>
> __be32 clusterip; /* the IP address */
> u_int8_t clustermac[ETH_ALEN]; /* the MAC address */
> - struct net_device *dev; /* device */
> + int ifindex; /* device ifindex */
> u_int16_t num_total_nodes; /* total number of nodes */
> unsigned long local_nodes; /* node number array */
>
> @@ -98,19 +98,23 @@ clusterip_config_put(struct clusterip_config *c)
> * entry(rule) is removed, remove the config from lists, but don't free it
> * yet, since proc-files could still be holding references */
> static inline void
> -clusterip_config_entry_put(struct clusterip_config *c)
> +clusterip_config_entry_put(struct net *net, struct clusterip_config *c)
> {
> - struct net *net = dev_net(c->dev);
> struct clusterip_net *cn = net_generic(net, clusterip_net_id);
>
> local_bh_disable();
> if (refcount_dec_and_lock(&c->entries, &cn->lock)) {
> + struct net_device *dev;
> +
> list_del_rcu(&c->list);
> spin_unlock(&cn->lock);
> local_bh_enable();
>
> - dev_mc_del(c->dev, c->clustermac);
> - dev_put(c->dev);
> + dev = dev_get_by_index(net, c->ifindex);
> + if (dev) {
> + dev_mc_del(dev, c->clustermac);
> + dev_put(dev);
> + }
>
> /* In case anyone still accesses the file, the open/close
> * functions are also incrementing the refcount on their own,
> @@ -182,7 +186,7 @@ clusterip_config_init(const struct ipt_clusterip_tgt_info *i, __be32 ip,
> if (!c)
> return ERR_PTR(-ENOMEM);
>
> - c->dev = dev;
> + c->ifindex = dev->ifindex;
> c->clusterip = ip;
> memcpy(&c->clustermac, &i->clustermac, ETH_ALEN);
> c->num_total_nodes = i->num_total_nodes;
> @@ -427,12 +431,14 @@ static int clusterip_tg_check(const struct xt_tgchk_param *par)
> }
>
> config = clusterip_config_init(cipinfo,
> - e->ip.dst.s_addr, dev);
> + e->ip.dst.s_addr, dev);
> if (IS_ERR(config)) {
> dev_put(dev);
> return PTR_ERR(config);
> }
> - dev_mc_add(config->dev, config->clustermac);
> +
> + dev_mc_add(dev, config->clustermac);
> + dev_put(dev);
> }
> }
> cipinfo->config = config;
> @@ -458,7 +464,7 @@ static void clusterip_tg_destroy(const struct xt_tgdtor_param *par)
>
> /* if no more entries are referencing the config, remove it
> * from the list and destroy the proc entry */
> - clusterip_config_entry_put(cipinfo->config);
> + clusterip_config_entry_put(par->net, cipinfo->config);
>
> clusterip_config_put(cipinfo->config);
>
> @@ -558,10 +564,9 @@ arp_mangle(void *priv,
> * addresses on different interfacs. However, in the CLUSTERIP case
> * this wouldn't work, since we didn't subscribe the mcast group on
> * other interfaces */
> - if (c->dev != state->out) {
> - pr_debug("not mangling arp reply on different "
> - "interface: cip'%s'-skb'%s'\n",
> - c->dev->name, state->out->name);
> + if (c->ifindex != state->out->ifindex) {
I'm missing the code to register_netdevice_notifier() so we can
refresh c->ifindex.
Just like we do in net/netfilter/xt_TEE.c
^ permalink raw reply
* [PATCH net-next 1/4] net: phy: marvell: #defines for copper and fibre pages
From: Andrew Lunn @ 2017-05-23 21:24 UTC (permalink / raw)
To: David Miller; +Cc: Florian Fainelli, netdev, Andrew Lunn
In-Reply-To: <1495574697-501-1-git-send-email-andrew@lunn.ch>
Replace magic numbers for PHY pages with symbolic names.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
drivers/net/phy/marvell.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index 88cd97b44ba6..bb067026353a 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -650,7 +650,7 @@ static int m88e1116r_config_init(struct phy_device *phydev)
mdelay(500);
- err = marvell_set_page(phydev, 0);
+ err = marvell_set_page(phydev, MII_M1111_COPPER);
if (err < 0)
return err;
@@ -662,7 +662,7 @@ static int m88e1116r_config_init(struct phy_device *phydev)
if (err < 0)
return err;
- err = marvell_set_page(phydev, 2);
+ err = marvell_set_page(phydev, MII_88E1121_PHY_MSCR_PAGE);
if (err < 0)
return err;
temp = phy_read(phydev, MII_M1116R_CONTROL_REG_MAC);
@@ -671,7 +671,7 @@ static int m88e1116r_config_init(struct phy_device *phydev)
err = phy_write(phydev, MII_M1116R_CONTROL_REG_MAC, temp);
if (err < 0)
return err;
- err = marvell_set_page(phydev, 0);
+ err = marvell_set_page(phydev, MII_M1111_COPPER);
if (err < 0)
return err;
@@ -892,7 +892,7 @@ static int m88e1510_config_init(struct phy_device *phydev)
return err;
/* Reset page selection */
- err = marvell_set_page(phydev, 0);
+ err = marvell_set_page(phydev, MII_M1111_COPPER);
if (err < 0)
return err;
}
@@ -922,7 +922,7 @@ static int m88e1118_config_init(struct phy_device *phydev)
int err;
/* Change address */
- err = marvell_set_page(phydev, 2);
+ err = marvell_set_page(phydev, MII_88E1121_PHY_MSCR_PAGE);
if (err < 0)
return err;
@@ -932,7 +932,7 @@ static int m88e1118_config_init(struct phy_device *phydev)
return err;
/* Change address */
- err = marvell_set_page(phydev, 3);
+ err = marvell_set_page(phydev, MII_88E1318S_PHY_LED_PAGE);
if (err < 0)
return err;
@@ -949,7 +949,7 @@ static int m88e1118_config_init(struct phy_device *phydev)
return err;
/* Reset address */
- err = marvell_set_page(phydev, 0);
+ err = marvell_set_page(phydev, MII_M1111_COPPER);
if (err < 0)
return err;
@@ -961,7 +961,7 @@ static int m88e1149_config_init(struct phy_device *phydev)
int err;
/* Change address */
- err = marvell_set_page(phydev, 2);
+ err = marvell_set_page(phydev, MII_88E1121_PHY_MSCR_PAGE);
if (err < 0)
return err;
@@ -975,7 +975,7 @@ static int m88e1149_config_init(struct phy_device *phydev)
return err;
/* Reset address */
- err = marvell_set_page(phydev, 0);
+ err = marvell_set_page(phydev, MII_M1111_COPPER);
if (err < 0)
return err;
@@ -1409,7 +1409,7 @@ static void m88e1318_get_wol(struct phy_device *phydev,
MII_88E1318S_PHY_WOL_CTRL_MAGIC_PACKET_MATCH_ENABLE)
wol->wolopts |= WAKE_MAGIC;
- if (marvell_set_page(phydev, 0x00) < 0)
+ if (marvell_set_page(phydev, MII_M1111_COPPER) < 0)
return;
}
@@ -1422,7 +1422,7 @@ static int m88e1318_set_wol(struct phy_device *phydev,
if (wol->wolopts & WAKE_MAGIC) {
/* Explicitly switch to page 0x00, just to be sure */
- err = marvell_set_page(phydev, 0x00);
+ err = marvell_set_page(phydev, MII_M1111_COPPER);
if (err < 0)
return err;
--
2.11.0
^ permalink raw reply related
* [PATCH net-next 2/4] net: phy: marvell: More hidden page changes refactored
From: Andrew Lunn @ 2017-05-23 21:24 UTC (permalink / raw)
To: David Miller; +Cc: Florian Fainelli, netdev, Andrew Lunn
In-Reply-To: <1495574697-501-1-git-send-email-andrew@lunn.ch>
EXT_ADDR_PAGE is the same meaning as MII_MARVELL_PHY_PAGE, i.e. change
page. Replace it will calls to the helpers.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
drivers/net/phy/marvell.c | 62 +++++++++++++++++++++++++++++++++++------------
1 file changed, 46 insertions(+), 16 deletions(-)
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index bb067026353a..44ea1dd89ce5 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -54,7 +54,6 @@
#define MII_M1011_PHY_SCR_MDI_X 0x0020
#define MII_M1011_PHY_SCR_AUTO_CROSS 0x0060
-#define MII_M1145_PHY_EXT_ADDR_PAGE 0x16
#define MII_M1145_PHY_EXT_SR 0x1b
#define MII_M1145_PHY_EXT_CR 0x14
#define MII_M1145_RGMII_RX_DELAY 0x0080
@@ -92,6 +91,7 @@
#define MII_88E1121_PHY_MSCR_TX_DELAY BIT(4)
#define MII_88E1121_PHY_MSCR_DELAY_MASK (~(0x3 << 4))
+#define MII_88E1121_MISC_TEST_PAGE 6
#define MII_88E1121_MISC_TEST 0x1a
#define MII_88E1510_MISC_TEST_TEMP_THRESHOLD_MASK 0x1f00
#define MII_88E1510_MISC_TEST_TEMP_THRESHOLD_SHIFT 8
@@ -760,11 +760,7 @@ static int m88e1111_config_init_sgmii(struct phy_device *phydev)
return err;
/* make sure copper is selected */
- err = phy_read(phydev, MII_M1145_PHY_EXT_ADDR_PAGE);
- if (err < 0)
- return err;
-
- return phy_write(phydev, MII_M1145_PHY_EXT_ADDR_PAGE, err & (~0xff));
+ return marvell_set_page(phydev, MII_M1111_COPPER);
}
static int m88e1111_config_init_rtbi(struct phy_device *phydev)
@@ -1556,12 +1552,19 @@ static int m88e1121_get_temp(struct phy_device *phydev, long *temp)
{
int ret;
int val;
+ int oldpage;
*temp = 0;
mutex_lock(&phydev->lock);
- ret = phy_write(phydev, MII_M1145_PHY_EXT_ADDR_PAGE, 0x6);
+ oldpage = marvell_get_page(phydev);
+ if (oldpage < 0) {
+ mutex_unlock(&phydev->lock);
+ return oldpage;
+ }
+
+ ret = marvell_set_page(phydev, MII_88E1121_MISC_TEST_PAGE);
if (ret < 0)
goto error;
@@ -1593,7 +1596,7 @@ static int m88e1121_get_temp(struct phy_device *phydev, long *temp)
*temp = ((val & MII_88E1121_MISC_TEST_TEMP_MASK) - 5) * 5000;
error:
- phy_write(phydev, MII_M1145_PHY_EXT_ADDR_PAGE, 0x0);
+ marvell_set_page(phydev, oldpage);
mutex_unlock(&phydev->lock);
return ret;
@@ -1671,12 +1674,19 @@ static const struct hwmon_chip_info m88e1121_hwmon_chip_info = {
static int m88e1510_get_temp(struct phy_device *phydev, long *temp)
{
int ret;
+ int oldpage;
*temp = 0;
mutex_lock(&phydev->lock);
- ret = phy_write(phydev, MII_M1145_PHY_EXT_ADDR_PAGE, 0x6);
+ oldpage = marvell_get_page(phydev);
+ if (oldpage < 0) {
+ mutex_unlock(&phydev->lock);
+ return oldpage;
+ }
+
+ ret = marvell_set_page(phydev, MII_88E1121_MISC_TEST_PAGE);
if (ret < 0)
goto error;
@@ -1687,7 +1697,7 @@ static int m88e1510_get_temp(struct phy_device *phydev, long *temp)
*temp = ((ret & MII_88E1510_TEMP_SENSOR_MASK) - 25) * 1000;
error:
- phy_write(phydev, MII_M1145_PHY_EXT_ADDR_PAGE, 0x0);
+ marvell_set_page(phydev, oldpage);
mutex_unlock(&phydev->lock);
return ret;
@@ -1696,12 +1706,18 @@ static int m88e1510_get_temp(struct phy_device *phydev, long *temp)
int m88e1510_get_temp_critical(struct phy_device *phydev, long *temp)
{
int ret;
+ int oldpage;
*temp = 0;
mutex_lock(&phydev->lock);
+ oldpage = marvell_get_page(phydev);
+ if (oldpage < 0) {
+ mutex_unlock(&phydev->lock);
+ return oldpage;
+ }
- ret = phy_write(phydev, MII_M1145_PHY_EXT_ADDR_PAGE, 0x6);
+ ret = marvell_set_page(phydev, MII_88E1121_MISC_TEST_PAGE);
if (ret < 0)
goto error;
@@ -1715,7 +1731,7 @@ int m88e1510_get_temp_critical(struct phy_device *phydev, long *temp)
*temp *= 1000;
error:
- phy_write(phydev, MII_M1145_PHY_EXT_ADDR_PAGE, 0x0);
+ marvell_set_page(phydev, oldpage);
mutex_unlock(&phydev->lock);
return ret;
@@ -1724,10 +1740,17 @@ int m88e1510_get_temp_critical(struct phy_device *phydev, long *temp)
int m88e1510_set_temp_critical(struct phy_device *phydev, long temp)
{
int ret;
+ int oldpage;
mutex_lock(&phydev->lock);
- ret = phy_write(phydev, MII_M1145_PHY_EXT_ADDR_PAGE, 0x6);
+ oldpage = marvell_get_page(phydev);
+ if (oldpage < 0) {
+ mutex_unlock(&phydev->lock);
+ return oldpage;
+ }
+
+ ret = marvell_set_page(phydev, MII_88E1121_MISC_TEST_PAGE);
if (ret < 0)
goto error;
@@ -1742,7 +1765,7 @@ int m88e1510_set_temp_critical(struct phy_device *phydev, long temp)
(temp << MII_88E1510_MISC_TEST_TEMP_THRESHOLD_SHIFT));
error:
- phy_write(phydev, MII_M1145_PHY_EXT_ADDR_PAGE, 0x0);
+ marvell_set_page(phydev, oldpage);
mutex_unlock(&phydev->lock);
return ret;
@@ -1751,12 +1774,19 @@ int m88e1510_set_temp_critical(struct phy_device *phydev, long temp)
int m88e1510_get_temp_alarm(struct phy_device *phydev, long *alarm)
{
int ret;
+ int oldpage;
*alarm = false;
mutex_lock(&phydev->lock);
- ret = phy_write(phydev, MII_M1145_PHY_EXT_ADDR_PAGE, 0x6);
+ oldpage = marvell_get_page(phydev);
+ if (oldpage < 0) {
+ mutex_unlock(&phydev->lock);
+ return oldpage;
+ }
+
+ ret = marvell_set_page(phydev, MII_88E1121_MISC_TEST_PAGE);
if (ret < 0)
goto error;
@@ -1766,7 +1796,7 @@ int m88e1510_get_temp_alarm(struct phy_device *phydev, long *alarm)
*alarm = !!(ret & MII_88E1510_MISC_TEST_TEMP_IRQ);
error:
- phy_write(phydev, MII_M1145_PHY_EXT_ADDR_PAGE, 0x0);
+ marvell_set_page(phydev, oldpage);
mutex_unlock(&phydev->lock);
return ret;
--
2.11.0
^ permalink raw reply related
* [PATCH net-next 4/4] net: phy: marvell: Uniform page names
From: Andrew Lunn @ 2017-05-23 21:24 UTC (permalink / raw)
To: David Miller; +Cc: Florian Fainelli, netdev, Andrew Lunn
In-Reply-To: <1495574697-501-1-git-send-email-andrew@lunn.ch>
Bring all the page names together, remove the repeats, and make them
uniform.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
drivers/net/phy/marvell.c | 94 +++++++++++++++++++++++------------------------
1 file changed, 46 insertions(+), 48 deletions(-)
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index ab00609af2be..0569614ec236 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -41,6 +41,12 @@
#include <linux/uaccess.h>
#define MII_MARVELL_PHY_PAGE 22
+#define MII_MARVELL_COPPER_PAGE 0x00
+#define MII_MARVELL_FIBER_PAGE 0x01
+#define MII_MARVELL_MSCR_PAGE 0x02
+#define MII_MARVELL_LED_PAGE 0x03
+#define MII_MARVELL_MISC_TEST_PAGE 0x06
+#define MII_MARVELL_WOL_PAGE 0x11
#define MII_M1011_IEVENT 0x13
#define MII_M1011_IEVENT_CLEAR 0x0000
@@ -82,16 +88,11 @@
#define MII_M1111_HWCFG_FIBER_COPPER_AUTO 0x8000
#define MII_M1111_HWCFG_FIBER_COPPER_RES 0x2000
-#define MII_M1111_COPPER 0
-#define MII_M1111_FIBER 1
-
-#define MII_88E1121_PHY_MSCR_PAGE 2
#define MII_88E1121_PHY_MSCR_REG 21
#define MII_88E1121_PHY_MSCR_RX_DELAY BIT(5)
#define MII_88E1121_PHY_MSCR_TX_DELAY BIT(4)
#define MII_88E1121_PHY_MSCR_DELAY_MASK (~(0x3 << 4))
-#define MII_88E1121_MISC_TEST_PAGE 6
#define MII_88E1121_MISC_TEST 0x1a
#define MII_88E1510_MISC_TEST_TEMP_THRESHOLD_MASK 0x1f00
#define MII_88E1510_MISC_TEST_TEMP_THRESHOLD_SHIFT 8
@@ -112,7 +113,6 @@
#define MII_88E1318S_PHY_CSIER_WOL_EIE BIT(7)
/* LED Timer Control Register */
-#define MII_88E1318S_PHY_LED_PAGE 0x03
#define MII_88E1318S_PHY_LED_TCR 0x12
#define MII_88E1318S_PHY_LED_TCR_FORCE_INT BIT(15)
#define MII_88E1318S_PHY_LED_TCR_INTn_ENABLE BIT(7)
@@ -123,13 +123,11 @@
#define MII_88E1318S_PHY_MAGIC_PACKET_WORD1 0x18
#define MII_88E1318S_PHY_MAGIC_PACKET_WORD0 0x19
-#define MII_88E1318S_PHY_WOL_PAGE 0x11
#define MII_88E1318S_PHY_WOL_CTRL 0x10
#define MII_88E1318S_PHY_WOL_CTRL_CLEAR_WOL_STATUS BIT(12)
#define MII_88E1318S_PHY_WOL_CTRL_MAGIC_PACKET_MATCH_ENABLE BIT(14)
#define MII_88E1121_PHY_LED_CTRL 16
-#define MII_88E1121_PHY_LED_PAGE 3
#define MII_88E1121_PHY_LED_DEF 0x0030
#define MII_M1011_PHY_STATUS 0x11
@@ -465,7 +463,7 @@ static int m88e1121_config_aneg(struct phy_device *phydev)
{
int err, oldpage, mscr;
- oldpage = marvell_get_set_page(phydev, MII_88E1121_PHY_MSCR_PAGE);
+ oldpage = marvell_get_set_page(phydev, MII_MARVELL_MSCR_PAGE);
if (oldpage < 0)
return oldpage;
@@ -504,7 +502,7 @@ static int m88e1318_config_aneg(struct phy_device *phydev)
{
int err, oldpage, mscr;
- oldpage = marvell_get_set_page(phydev, MII_88E1121_PHY_MSCR_PAGE);
+ oldpage = marvell_get_set_page(phydev, MII_MARVELL_MSCR_PAGE);
if (oldpage < 0)
return oldpage;
@@ -615,7 +613,7 @@ static int m88e1510_config_aneg(struct phy_device *phydev)
{
int err;
- err = marvell_set_page(phydev, MII_M1111_COPPER);
+ err = marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE);
if (err < 0)
goto error;
@@ -625,7 +623,7 @@ static int m88e1510_config_aneg(struct phy_device *phydev)
goto error;
/* Then the fiber link */
- err = marvell_set_page(phydev, MII_M1111_FIBER);
+ err = marvell_set_page(phydev, MII_MARVELL_FIBER_PAGE);
if (err < 0)
goto error;
@@ -633,10 +631,10 @@ static int m88e1510_config_aneg(struct phy_device *phydev)
if (err < 0)
goto error;
- return marvell_set_page(phydev, MII_M1111_COPPER);
+ return marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE);
error:
- marvell_set_page(phydev, MII_M1111_COPPER);
+ marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE);
return err;
}
@@ -659,7 +657,7 @@ static int m88e1116r_config_init(struct phy_device *phydev)
mdelay(500);
- err = marvell_set_page(phydev, MII_M1111_COPPER);
+ err = marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE);
if (err < 0)
return err;
@@ -671,7 +669,7 @@ static int m88e1116r_config_init(struct phy_device *phydev)
if (err < 0)
return err;
- err = marvell_set_page(phydev, MII_88E1121_PHY_MSCR_PAGE);
+ err = marvell_set_page(phydev, MII_MARVELL_MSCR_PAGE);
if (err < 0)
return err;
temp = phy_read(phydev, MII_M1116R_CONTROL_REG_MAC);
@@ -680,7 +678,7 @@ static int m88e1116r_config_init(struct phy_device *phydev)
err = phy_write(phydev, MII_M1116R_CONTROL_REG_MAC, temp);
if (err < 0)
return err;
- err = marvell_set_page(phydev, MII_M1111_COPPER);
+ err = marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE);
if (err < 0)
return err;
@@ -769,7 +767,7 @@ static int m88e1111_config_init_sgmii(struct phy_device *phydev)
return err;
/* make sure copper is selected */
- return marvell_set_page(phydev, MII_M1111_COPPER);
+ return marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE);
}
static int m88e1111_config_init_rtbi(struct phy_device *phydev)
@@ -852,7 +850,7 @@ static int m88e1121_config_init(struct phy_device *phydev)
{
int err, oldpage;
- oldpage = marvell_get_set_page(phydev, MII_88E1121_PHY_LED_PAGE);
+ oldpage = marvell_get_set_page(phydev, MII_MARVELL_LED_PAGE);
if (oldpage < 0)
return oldpage;
@@ -895,7 +893,7 @@ static int m88e1510_config_init(struct phy_device *phydev)
return err;
/* Reset page selection */
- err = marvell_set_page(phydev, MII_M1111_COPPER);
+ err = marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE);
if (err < 0)
return err;
}
@@ -925,7 +923,7 @@ static int m88e1118_config_init(struct phy_device *phydev)
int err;
/* Change address */
- err = marvell_set_page(phydev, MII_88E1121_PHY_MSCR_PAGE);
+ err = marvell_set_page(phydev, MII_MARVELL_MSCR_PAGE);
if (err < 0)
return err;
@@ -935,7 +933,7 @@ static int m88e1118_config_init(struct phy_device *phydev)
return err;
/* Change address */
- err = marvell_set_page(phydev, MII_88E1318S_PHY_LED_PAGE);
+ err = marvell_set_page(phydev, MII_MARVELL_LED_PAGE);
if (err < 0)
return err;
@@ -952,7 +950,7 @@ static int m88e1118_config_init(struct phy_device *phydev)
return err;
/* Reset address */
- err = marvell_set_page(phydev, MII_M1111_COPPER);
+ err = marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE);
if (err < 0)
return err;
@@ -964,7 +962,7 @@ static int m88e1149_config_init(struct phy_device *phydev)
int err;
/* Change address */
- err = marvell_set_page(phydev, MII_88E1121_PHY_MSCR_PAGE);
+ err = marvell_set_page(phydev, MII_MARVELL_MSCR_PAGE);
if (err < 0)
return err;
@@ -978,7 +976,7 @@ static int m88e1149_config_init(struct phy_device *phydev)
return err;
/* Reset address */
- err = marvell_set_page(phydev, MII_M1111_COPPER);
+ err = marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE);
if (err < 0)
return err;
@@ -1248,7 +1246,7 @@ static int marvell_read_status_page(struct phy_device *phydev, int page)
/* Detect and update the link, but return if there
* was an error
*/
- if (page == MII_M1111_FIBER)
+ if (page == MII_MARVELL_FIBER_PAGE)
fiber = 1;
else
fiber = 0;
@@ -1281,11 +1279,11 @@ static int marvell_read_status(struct phy_device *phydev)
/* Check the fiber mode first */
if (phydev->supported & SUPPORTED_FIBRE &&
phydev->interface != PHY_INTERFACE_MODE_SGMII) {
- err = marvell_set_page(phydev, MII_M1111_FIBER);
+ err = marvell_set_page(phydev, MII_MARVELL_FIBER_PAGE);
if (err < 0)
goto error;
- err = marvell_read_status_page(phydev, MII_M1111_FIBER);
+ err = marvell_read_status_page(phydev, MII_MARVELL_FIBER_PAGE);
if (err < 0)
goto error;
@@ -1300,15 +1298,15 @@ static int marvell_read_status(struct phy_device *phydev)
return 0;
/* If fiber link is down, check and save copper mode state */
- err = marvell_set_page(phydev, MII_M1111_COPPER);
+ err = marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE);
if (err < 0)
goto error;
}
- return marvell_read_status_page(phydev, MII_M1111_COPPER);
+ return marvell_read_status_page(phydev, MII_MARVELL_COPPER_PAGE);
error:
- marvell_set_page(phydev, MII_M1111_COPPER);
+ marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE);
return err;
}
@@ -1323,7 +1321,7 @@ static int marvell_suspend(struct phy_device *phydev)
/* Suspend the fiber mode first */
if (!(phydev->supported & SUPPORTED_FIBRE)) {
- err = marvell_set_page(phydev, MII_M1111_FIBER);
+ err = marvell_set_page(phydev, MII_MARVELL_FIBER_PAGE);
if (err < 0)
goto error;
@@ -1333,7 +1331,7 @@ static int marvell_suspend(struct phy_device *phydev)
goto error;
/* Then, the copper link */
- err = marvell_set_page(phydev, MII_M1111_COPPER);
+ err = marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE);
if (err < 0)
goto error;
}
@@ -1342,7 +1340,7 @@ static int marvell_suspend(struct phy_device *phydev)
return genphy_suspend(phydev);
error:
- marvell_set_page(phydev, MII_M1111_COPPER);
+ marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE);
return err;
}
@@ -1357,7 +1355,7 @@ static int marvell_resume(struct phy_device *phydev)
/* Resume the fiber mode first */
if (!(phydev->supported & SUPPORTED_FIBRE)) {
- err = marvell_set_page(phydev, MII_M1111_FIBER);
+ err = marvell_set_page(phydev, MII_MARVELL_FIBER_PAGE);
if (err < 0)
goto error;
@@ -1367,7 +1365,7 @@ static int marvell_resume(struct phy_device *phydev)
goto error;
/* Then, the copper link */
- err = marvell_set_page(phydev, MII_M1111_COPPER);
+ err = marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE);
if (err < 0)
goto error;
}
@@ -1376,7 +1374,7 @@ static int marvell_resume(struct phy_device *phydev)
return genphy_resume(phydev);
error:
- marvell_set_page(phydev, MII_M1111_COPPER);
+ marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE);
return err;
}
@@ -1405,14 +1403,14 @@ static void m88e1318_get_wol(struct phy_device *phydev,
wol->supported = WAKE_MAGIC;
wol->wolopts = 0;
- if (marvell_set_page(phydev, MII_88E1318S_PHY_WOL_PAGE) < 0)
+ if (marvell_set_page(phydev, MII_MARVELL_WOL_PAGE) < 0)
return;
if (phy_read(phydev, MII_88E1318S_PHY_WOL_CTRL) &
MII_88E1318S_PHY_WOL_CTRL_MAGIC_PACKET_MATCH_ENABLE)
wol->wolopts |= WAKE_MAGIC;
- if (marvell_set_page(phydev, MII_M1111_COPPER) < 0)
+ if (marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE) < 0)
return;
}
@@ -1425,7 +1423,7 @@ static int m88e1318_set_wol(struct phy_device *phydev,
if (wol->wolopts & WAKE_MAGIC) {
/* Explicitly switch to page 0x00, just to be sure */
- err = marvell_set_page(phydev, MII_M1111_COPPER);
+ err = marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE);
if (err < 0)
return err;
@@ -1436,7 +1434,7 @@ static int m88e1318_set_wol(struct phy_device *phydev,
if (err < 0)
return err;
- err = marvell_set_page(phydev, MII_88E1318S_PHY_LED_PAGE);
+ err = marvell_set_page(phydev, MII_MARVELL_LED_PAGE);
if (err < 0)
return err;
@@ -1449,7 +1447,7 @@ static int m88e1318_set_wol(struct phy_device *phydev,
if (err < 0)
return err;
- err = marvell_set_page(phydev, MII_88E1318S_PHY_WOL_PAGE);
+ err = marvell_set_page(phydev, MII_MARVELL_WOL_PAGE);
if (err < 0)
return err;
@@ -1478,7 +1476,7 @@ static int m88e1318_set_wol(struct phy_device *phydev,
if (err < 0)
return err;
} else {
- err = marvell_set_page(phydev, MII_88E1318S_PHY_WOL_PAGE);
+ err = marvell_set_page(phydev, MII_MARVELL_WOL_PAGE);
if (err < 0)
return err;
@@ -1564,7 +1562,7 @@ static int m88e1121_get_temp(struct phy_device *phydev, long *temp)
mutex_lock(&phydev->lock);
- oldpage = marvell_get_set_page(phydev, MII_88E1121_MISC_TEST_PAGE);
+ oldpage = marvell_get_set_page(phydev, MII_MARVELL_MISC_TEST_PAGE);
if (oldpage < 0) {
mutex_unlock(&phydev->lock);
return oldpage;
@@ -1682,7 +1680,7 @@ static int m88e1510_get_temp(struct phy_device *phydev, long *temp)
mutex_lock(&phydev->lock);
- oldpage = marvell_get_set_page(phydev, MII_88E1121_MISC_TEST_PAGE);
+ oldpage = marvell_get_set_page(phydev, MII_MARVELL_MISC_TEST_PAGE);
if (oldpage < 0) {
mutex_unlock(&phydev->lock);
return oldpage;
@@ -1710,7 +1708,7 @@ int m88e1510_get_temp_critical(struct phy_device *phydev, long *temp)
mutex_lock(&phydev->lock);
- oldpage = marvell_get_set_page(phydev, MII_88E1121_MISC_TEST_PAGE);
+ oldpage = marvell_get_set_page(phydev, MII_MARVELL_MISC_TEST_PAGE);
if (oldpage < 0) {
mutex_unlock(&phydev->lock);
return oldpage;
@@ -1739,7 +1737,7 @@ int m88e1510_set_temp_critical(struct phy_device *phydev, long temp)
mutex_lock(&phydev->lock);
- oldpage = marvell_get_set_page(phydev, MII_88E1121_MISC_TEST_PAGE);
+ oldpage = marvell_get_set_page(phydev, MII_MARVELL_MISC_TEST_PAGE);
if (oldpage < 0) {
mutex_unlock(&phydev->lock);
return oldpage;
@@ -1771,7 +1769,7 @@ int m88e1510_get_temp_alarm(struct phy_device *phydev, long *alarm)
mutex_lock(&phydev->lock);
- oldpage = marvell_get_set_page(phydev, MII_88E1121_MISC_TEST_PAGE);
+ oldpage = marvell_get_set_page(phydev, MII_MARVELL_MISC_TEST_PAGE);
if (oldpage < 0) {
mutex_unlock(&phydev->lock);
return oldpage;
--
2.11.0
^ permalink raw reply related
* [PATCH net-next 3/4] net: phy: marvell: helper to get and set page
From: Andrew Lunn @ 2017-05-23 21:24 UTC (permalink / raw)
To: David Miller; +Cc: Florian Fainelli, netdev, Andrew Lunn
In-Reply-To: <1495574697-501-1-git-send-email-andrew@lunn.ch>
There is a common pattern of first reading the currently selected page
and then changing to another page. Add a helper to do this.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
drivers/net/phy/marvell.c | 75 ++++++++++++++++++++---------------------------
1 file changed, 31 insertions(+), 44 deletions(-)
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index 44ea1dd89ce5..ab00609af2be 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -199,6 +199,19 @@ static int marvell_set_page(struct phy_device *phydev, int page)
return phy_write(phydev, MII_MARVELL_PHY_PAGE, page);
}
+static int marvell_get_set_page(struct phy_device *phydev, int page)
+{
+ int oldpage = marvell_get_page(phydev);
+
+ if (oldpage < 0)
+ return oldpage;
+
+ if (page != oldpage)
+ return marvell_set_page(phydev, page);
+
+ return 0;
+}
+
static int marvell_ack_interrupt(struct phy_device *phydev)
{
int err;
@@ -452,11 +465,9 @@ static int m88e1121_config_aneg(struct phy_device *phydev)
{
int err, oldpage, mscr;
- oldpage = marvell_get_page(phydev);
-
- err = marvell_set_page(phydev, MII_88E1121_PHY_MSCR_PAGE);
- if (err < 0)
- return err;
+ oldpage = marvell_get_set_page(phydev, MII_88E1121_PHY_MSCR_PAGE);
+ if (oldpage < 0)
+ return oldpage;
if (phy_interface_is_rgmii(phydev)) {
mscr = phy_read(phydev, MII_88E1121_PHY_MSCR_REG) &
@@ -493,11 +504,9 @@ static int m88e1318_config_aneg(struct phy_device *phydev)
{
int err, oldpage, mscr;
- oldpage = marvell_get_page(phydev);
-
- err = marvell_set_page(phydev, MII_88E1121_PHY_MSCR_PAGE);
- if (err < 0)
- return err;
+ oldpage = marvell_get_set_page(phydev, MII_88E1121_PHY_MSCR_PAGE);
+ if (oldpage < 0)
+ return oldpage;
mscr = phy_read(phydev, MII_88E1318S_PHY_MSCR1_REG);
mscr |= MII_88E1318S_PHY_MSCR1_PAD_ODD;
@@ -843,11 +852,9 @@ static int m88e1121_config_init(struct phy_device *phydev)
{
int err, oldpage;
- oldpage = marvell_get_page(phydev);
-
- err = marvell_set_page(phydev, MII_88E1121_PHY_LED_PAGE);
- if (err < 0)
- return err;
+ oldpage = marvell_get_set_page(phydev, MII_88E1121_PHY_LED_PAGE);
+ if (oldpage < 0)
+ return oldpage;
/* Default PHY LED config: LED[0] .. Link, LED[1] .. Activity */
err = phy_write(phydev, MII_88E1121_PHY_LED_CTRL,
@@ -1516,12 +1523,11 @@ static u64 marvell_get_stat(struct phy_device *phydev, int i)
{
struct marvell_hw_stat stat = marvell_hw_stats[i];
struct marvell_priv *priv = phydev->priv;
- int err, oldpage, val;
+ int oldpage, val;
u64 ret;
- oldpage = marvell_get_page(phydev);
- err = marvell_set_page(phydev, stat.page);
- if (err < 0)
+ oldpage = marvell_get_set_page(phydev, stat.page);
+ if (oldpage < 0)
return UINT64_MAX;
val = phy_read(phydev, stat.reg);
@@ -1558,16 +1564,12 @@ static int m88e1121_get_temp(struct phy_device *phydev, long *temp)
mutex_lock(&phydev->lock);
- oldpage = marvell_get_page(phydev);
+ oldpage = marvell_get_set_page(phydev, MII_88E1121_MISC_TEST_PAGE);
if (oldpage < 0) {
mutex_unlock(&phydev->lock);
return oldpage;
}
- ret = marvell_set_page(phydev, MII_88E1121_MISC_TEST_PAGE);
- if (ret < 0)
- goto error;
-
/* Enable temperature sensor */
ret = phy_read(phydev, MII_88E1121_MISC_TEST);
if (ret < 0)
@@ -1680,16 +1682,12 @@ static int m88e1510_get_temp(struct phy_device *phydev, long *temp)
mutex_lock(&phydev->lock);
- oldpage = marvell_get_page(phydev);
+ oldpage = marvell_get_set_page(phydev, MII_88E1121_MISC_TEST_PAGE);
if (oldpage < 0) {
mutex_unlock(&phydev->lock);
return oldpage;
}
- ret = marvell_set_page(phydev, MII_88E1121_MISC_TEST_PAGE);
- if (ret < 0)
- goto error;
-
ret = phy_read(phydev, MII_88E1510_TEMP_SENSOR);
if (ret < 0)
goto error;
@@ -1711,16 +1709,13 @@ int m88e1510_get_temp_critical(struct phy_device *phydev, long *temp)
*temp = 0;
mutex_lock(&phydev->lock);
- oldpage = marvell_get_page(phydev);
+
+ oldpage = marvell_get_set_page(phydev, MII_88E1121_MISC_TEST_PAGE);
if (oldpage < 0) {
mutex_unlock(&phydev->lock);
return oldpage;
}
- ret = marvell_set_page(phydev, MII_88E1121_MISC_TEST_PAGE);
- if (ret < 0)
- goto error;
-
ret = phy_read(phydev, MII_88E1121_MISC_TEST);
if (ret < 0)
goto error;
@@ -1744,16 +1739,12 @@ int m88e1510_set_temp_critical(struct phy_device *phydev, long temp)
mutex_lock(&phydev->lock);
- oldpage = marvell_get_page(phydev);
+ oldpage = marvell_get_set_page(phydev, MII_88E1121_MISC_TEST_PAGE);
if (oldpage < 0) {
mutex_unlock(&phydev->lock);
return oldpage;
}
- ret = marvell_set_page(phydev, MII_88E1121_MISC_TEST_PAGE);
- if (ret < 0)
- goto error;
-
ret = phy_read(phydev, MII_88E1121_MISC_TEST);
if (ret < 0)
goto error;
@@ -1780,16 +1771,12 @@ int m88e1510_get_temp_alarm(struct phy_device *phydev, long *alarm)
mutex_lock(&phydev->lock);
- oldpage = marvell_get_page(phydev);
+ oldpage = marvell_get_set_page(phydev, MII_88E1121_MISC_TEST_PAGE);
if (oldpage < 0) {
mutex_unlock(&phydev->lock);
return oldpage;
}
- ret = marvell_set_page(phydev, MII_88E1121_MISC_TEST_PAGE);
- if (ret < 0)
- goto error;
-
ret = phy_read(phydev, MII_88E1121_MISC_TEST);
if (ret < 0)
goto error;
--
2.11.0
^ permalink raw reply related
* Re: bond link state mismatch, rtnl_trylock() vs rtnl_lock()
From: Nithin Sujir @ 2017-05-23 21:10 UTC (permalink / raw)
To: Jay Vosburgh; +Cc: netdev
In-Reply-To: <1638.1495570412@famine>
On 5/23/2017 1:13 PM, Jay Vosburgh wrote:
> Nithin Sujir <nsujir@tintri.com> wrote:
>
>> Hi,
>> We're encountering a problem in 4.4 LTS where, rarely, the bond link state
>> is not updated when the slave link changes.
>>
>> I've traced the issue to the arp monitor unable to get the rtnl lock. The
>> sequence resulting in failure is as below.
>>
>> bond_loadbalance_arp_mon() periodically called, if slave link is _down_,
>> it checks if the slave is sending/receiving packets. If it is, it sets
>> flags to be processed later down the function for bond link
>> update. However, it sets the slave->link right away.
>>
>> if (slave->link != BOND_LINK_UP) {
>> if (bond_time_in_interval(bond, trans_start, 1) &&
>> bond_time_in_interval(bond, slave->last_rx,
>> 1)) {
>>
>> slave->link = BOND_LINK_UP;
>> slave_state_changed = 1;
>>
>>
>> Later down the function, it tries to get the rtnl_lock. If it doesn't get
>> it, it rearms and returns.
>>
>> if (do_failover || slave_state_changed) {
>> if (!rtnl_trylock())
>> goto re_arm; <-- returns here
>>
>> if (slave_state_changed) {
>> bond_slave_state_change(bond);
>>
>> This is the problem. The next time this function is called, the
>> slave->link is already marked UP. And we will never update the bond link
>> state to UP.
> This looks like an ARP monitor version of
>
> commit de77ecd4ef02ca783f7762e04e92b3d0964be66b
> Author: Mahesh Bandewar <maheshb@google.com>
> Date: Mon Mar 27 11:37:33 2017 -0700
>
> bonding: improve link-status update in mii-monitoring
>
> and probably needs a similar fix (possibly for both the
> loadbalance and active-backup ARP monitor cases).
Thanks for the explanation and the pointer to this patch. I will take a
look.
Thanks, Jay!
Nithin.
>> Changing the rtnl_trylock() -> rtnl_lock() _does_ fix the issue.
>>
>> Is this the right way to fix it? If it is, I can submit this formally.
> It's not the right way, unfortunately.
>
> The reason for the rtnl_trylock is that there's a possible race
> against bond_close() -> bond_work_cancel_all() trying to cancel the
> arp_work workqueue item while it's running. bond_close is called with
> RTNL held, so if it has RTNL and is waiting for the work function to
> complete, an rtnl_lock call here will deadlock. Some of the trylock
> calls in bonding are commented to this effect, but not this one.
>
> -J
>
>> What are the guidelines around using rtnl_lock() vs rtnl_trylock()? Some
>> places are using rtnl_lock() and other rtnl_trylock(). Sorry, I couldn't
>> find much via a google search or in Documentation/.
>>
>> Thanks,
>> Nithin.
>>
>> --------------------
>>
>> diff --git a/drivers/net/bonding/bond_main.c
>> b/drivers/net/bonding/bond_main.c
>> index 5dca77e..1f60503 100644
>> --- a/drivers/net/bonding/bond_main.c
>> +++ b/drivers/net/bonding/bond_main.c
>> @@ -2614,8 +2614,7 @@ static void bond_loadbalance_arp_mon(struct
>> work_struct *work)
>> rcu_read_unlock();
>>
>> if (do_failover || slave_state_changed) {
>> - if (!rtnl_trylock())
>> - goto re_arm;
>> + rtnl_lock();
>>
>> if (slave_state_changed) {
>> bond_slave_state_change(bond);
>>
>>
> ---
> -Jay Vosburgh, jay.vosburgh@canonical.com
^ permalink raw reply
* [PATCH v2 net-next 0/2] Start making stats consistent
From: Andrew Lunn @ 2017-05-23 21:10 UTC (permalink / raw)
To: David Miller; +Cc: Florian Fainelli, hayeswang, netdev, Andrew Lunn
It does not appear that the sysfs class net stats have been clearly
defined, resulting in different network interfaces implementing them
slightly different. Define that the rx_bytes and tx_bytes counters
should include the Ethernet header, data and FEC. Modify the r8152 USB
Ethernet driver to include the FEC in its counters.
Andrew Lunn (2):
Documentation: sysfs-class-net-statistics: Clarify rx_bytes and
tx_bytes
net: usb: r8152: Fix rx_bytes/tx_bytes to include FCS
Documentation/ABI/testing/sysfs-class-net-statistics | 6 ++++--
drivers/net/usb/r8152.c | 5 ++---
2 files changed, 6 insertions(+), 5 deletions(-)
--
2.11.0
^ permalink raw reply
* [PATCH v2 net-next 2/2] net: usb: r8152: Fix rx_bytes/tx_bytes to include FCS
From: Andrew Lunn @ 2017-05-23 21:10 UTC (permalink / raw)
To: David Miller; +Cc: Florian Fainelli, hayeswang, netdev, Andrew Lunn
In-Reply-To: <1495573858-32346-1-git-send-email-andrew@lunn.ch>
The statistics counters rx_bytes and tx_bytes don't include the
Ethernet Frame Check Sequence. Fix the rx_bytes/tx_bytes counters to
include the FCS, and include the received FCS in the skb.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
drivers/net/usb/r8152.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index ddc62cb69be8..4081a2cd8b1b 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -1228,7 +1228,7 @@ static void write_bulk_callback(struct urb *urb)
stats->tx_errors += agg->skb_num;
} else {
stats->tx_packets += agg->skb_num;
- stats->tx_bytes += agg->skb_len;
+ stats->tx_bytes += agg->skb_len + CRC_SIZE;
}
spin_lock(&tp->tx_lock);
@@ -1826,7 +1826,6 @@ static int rx_bottom(struct r8152 *tp, int budget)
if (urb->actual_length < len_used)
break;
- pkt_len -= CRC_SIZE;
rx_data += sizeof(struct rx_desc);
skb = napi_alloc_skb(napi, pkt_len);
@@ -1850,7 +1849,7 @@ static int rx_bottom(struct r8152 *tp, int budget)
}
find_next_rx:
- rx_data = rx_agg_align(rx_data + pkt_len + CRC_SIZE);
+ rx_data = rx_agg_align(rx_data + pkt_len);
rx_desc = (struct rx_desc *)rx_data;
len_used = (int)(rx_data - (u8 *)agg->head);
len_used += sizeof(struct rx_desc);
--
2.11.0
^ permalink raw reply related
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