* I wait for your prompt response.
From: Aziz Dake @ 2018-10-23 12:10 UTC (permalink / raw)
Good day,
I am Mr. Aziz Dake, from Burkina Faso a Minister confide on me to
look for foreign partner who will assist him to invest the sum of
Thirty Million Dollars ($30,000,000) in your country.
He has investment interest in mining, exotic properties for commercial
resident, development properties, hotels and any other viable
investment opportunities in your country based on your recommendation
will be highly welcomed.
Hence your co -operation is highly needed to actualize this investment project
I wait for your prompt response.
Sincerely yours
Mr Aziz Dake.
^ permalink raw reply
* Re: [PATCH v2] wireless: mark expected switch fall-throughs
From: Johannes Berg @ 2018-10-23 20:33 UTC (permalink / raw)
To: Gustavo A. R. Silva, David S. Miller
Cc: linux-wireless, netdev, linux-kernel, Kees Cook
In-Reply-To: <c2fad584-1705-a5f2-d63c-824e9b96cf50@embeddedor.com>
On Tue, 2018-10-23 at 12:58 +0200, Gustavo A. R. Silva wrote:
> On 10/23/18 10:59 AM, Gustavo A. R. Silva wrote:
> >
> > On 10/23/18 9:01 AM, Johannes Berg wrote:
> > > On Tue, 2018-10-23 at 02:13 +0200, Gustavo A. R. Silva wrote:
> > > > In preparation to enabling -Wimplicit-fallthrough, mark switch cases
> > > > where we are expecting to fall through.
> > > >
> > > > Warning level 3 was used: -Wimplicit-fallthrough=3
> > > >
> > > > This code was not tested and GCC 7.2.0 was used to compile it.
> > >
> > > Look, I'm not going to make this any clearer: I'm not applying patches
> > > like that where you've invested no effort whatsoever on verifying that
> > > they're correct.
> > >
> >
> > How do you suggest me to verify that every part is correct in this type
> > of patches?
> >
>
> BTW... I'm under the impression you think that I don't even look at
> the code. Is that correct?
That's what your commit log looks like, yes. This is your full commit
log:
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.
Warning level 3 was used: -Wimplicit-fallthrough=3
This code was not tested and GCC 7.2.0 was used to compile it.
For all I know, you could've run spatch to add the comments wherever
there was no break, and then compiled it once.
> I've been working on this for quite a while, and in every case I try to
> understand the code in terms of the context in which every warning is
> reported.
That's great.
> Here is a bug I found yesterday at drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>
> 5690 case WLAN_CIPHER_SUITE_CCMP:
> 5691 key->flags |= IEEE80211_KEY_FLAG_SW_MGMT_TX;
> 5692 break;
> 5693 case WLAN_CIPHER_SUITE_TKIP:
> 5694 key->flags |= IEEE80211_KEY_FLAG_GENERATE_MMIC;
> 5695 default:
> 5696 return -EOPNOTSUPP;
> 5697 }
Indeed, that looks like a bug, although kinda benign since it just means
TKIP will always use software crypto and TKIP is slow anyway ;-)
> I do this analysis for every warning. Now, when I say I haven't tested the code
> is because I don't have any log as evidence for anything. Not that I haven't put
> any effort on trying to understand it and its context. When I started working on
> this task there were more than 2000 of these issues, now there are around 600 left.
>
> I have fixed many bugs on the way, so a good amount of work is being invested on
> this, and it's paying off. :)
:-)
> Now, let me ask you this question:
>
> It would be easier for you to review this patch if I turn it into a series?
>
> I can do that without a problem.
I'd be happy if you were to actually just mention that you've looked at
them, and found them to be expected fall throughs. I'll still review
them, but without that information I feel like I'm doing the first round
of reviews this code ever got.
johannes
^ permalink raw reply
* Re: [RFC PATCH v2 00/10] udp: implement GRO support
From: Steffen Klassert @ 2018-10-23 12:10 UTC (permalink / raw)
To: Paolo Abeni; +Cc: netdev, Willem de Bruijn
In-Reply-To: <cover.1539957909.git.pabeni@redhat.com>
On Fri, Oct 19, 2018 at 04:25:10PM +0200, Paolo Abeni wrote:
> This series implements GRO support for UDP sockets, as the RX counterpart
> of commit bec1f6f69736 ("udp: generate gso with UDP_SEGMENT").
> The core functionality is implemented by the second patch, introducing a new
> sockopt to enable UDP_GRO, while patch 3 implements support for passing the
> segment size to the user space via a new cmsg.
> UDP GRO performs a socket lookup for each ingress packets and aggregate datagram
> directed to UDP GRO enabled sockets with constant l4 tuple.
>
> UDP GRO packets can land on non GRO-enabled sockets, e.g. due to iptables NAT
> rules, and that could potentially confuse existing applications.
>
> The solution adopted here is to de-segment the GRO packet before enqueuing
> as needed. Since we must cope with packet reinsertion after de-segmentation,
> the relevant code is factored-out in ipv4 and ipv6 specific helpers and exposed
> to UDP usage.
>
> While the current code can probably be improved, this safeguard ,implemented in
> the patches 4-7, allows future enachements to enable UDP GSO offload on more
> virtual devices eventually even on forwarded packets.
I was curious what I get with this when doing packet forwarding.
So I added forwarding support with the patch below (on top of
your patchset). While the packet processing could work the way
I do it in this patch, I'm not sure about the way I enable
UDP GRO on forwarding. Maybe there it a better way than just
do UDP GRO if forwarding is enabled on the receiving device.
Some quick benchmark numbers with UDP packet forwarding
(1460 byte packets) through two gateways:
net-next: 16.4 Gbps
net-next + UDP GRO: 20.3 Gbps
Subject: [PATCH RFC] udp: Allow gro for the forwarding path.
This patch adds a early route lookup to inet_gro_receive()
in case forwarding is enabled on the receiving device.
To be forwarded packets are allowed to enter the UDP
GRO handlers then.
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
include/linux/netdevice.h | 2 +-
include/net/dst.h | 1 +
net/core/dev.c | 6 ++++--
net/ipv4/af_inet.c | 12 ++++++++++++
net/ipv4/route.c | 1 +
net/ipv4/udp_offload.c | 11 +++++++++--
6 files changed, 28 insertions(+), 5 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index dc1d9ed33b31..2eb3fa960ad4 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2304,7 +2304,7 @@ struct napi_gro_cb {
/* Number of gro_receive callbacks this packet already went through */
u8 recursion_counter:4;
- /* 1 bit hole */
+ u8 is_fwd:1;
/* used to support CHECKSUM_COMPLETE for tunneling protocols */
__wsum csum;
diff --git a/include/net/dst.h b/include/net/dst.h
index 6cf0870414c7..01bdf825a6f9 100644
--- a/include/net/dst.h
+++ b/include/net/dst.h
@@ -54,6 +54,7 @@ struct dst_entry {
#define DST_XFRM_TUNNEL 0x0020
#define DST_XFRM_QUEUE 0x0040
#define DST_METADATA 0x0080
+#define DST_FWD 0x0100
/* A non-zero value of dst->obsolete forces by-hand validation
* of the route entry. Positive values are set by the generic
diff --git a/net/core/dev.c b/net/core/dev.c
index 022ad73d6253..c6aaffb99456 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5387,8 +5387,10 @@ static struct list_head *gro_list_prepare(struct napi_struct *napi,
diffs = (unsigned long)p->dev ^ (unsigned long)skb->dev;
diffs |= p->vlan_tci ^ skb->vlan_tci;
- diffs |= skb_metadata_dst_cmp(p, skb);
- diffs |= skb_metadata_differs(p, skb);
+ if (!NAPI_GRO_CB(p)->is_fwd) {
+ diffs |= skb_metadata_dst_cmp(p, skb);
+ diffs |= skb_metadata_differs(p, skb);
+ }
if (maclen == ETH_HLEN)
diffs |= compare_ether_header(skb_mac_header(p),
skb_mac_header(skb));
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 1fbe2f815474..6e4e8588c0b1 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -1479,8 +1479,20 @@ struct sk_buff *inet_gro_receive(struct list_head *head, struct sk_buff *skb)
NAPI_GRO_CB(p)->flush_id = flush_id;
else
NAPI_GRO_CB(p)->flush_id |= flush_id;
+
+ NAPI_GRO_CB(skb)->is_fwd = NAPI_GRO_CB(p)->is_fwd;
+
+ goto found;
}
+ if (IN_DEV_FORWARD(__in_dev_get_rcu(skb->dev))) {
+ if (!ip_route_input_noref(skb, iph->daddr, iph->saddr, iph->tos,
+ skb->dev)) {
+ if ((skb_dst(skb)->flags & DST_FWD))
+ NAPI_GRO_CB(skb)->is_fwd = 1;
+ }
+ }
+found:
NAPI_GRO_CB(skb)->is_atomic = !!(iph->frag_off & htons(IP_DF));
NAPI_GRO_CB(skb)->flush |= flush;
skb_set_network_header(skb, off);
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index c0a9d26c06ce..3e5b3f0be0f0 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1757,6 +1757,7 @@ static int __mkroute_input(struct sk_buff *skb,
rth->rt_is_input = 1;
RT_CACHE_STAT_INC(in_slow_tot);
+ rth->dst.flags |= DST_FWD;
rth->dst.input = ip_forward;
rt_set_nexthop(rth, daddr, res, fnhe, res->fi, res->type, itag,
diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
index d93c1e8097ba..c99f117bc44e 100644
--- a/net/ipv4/udp_offload.c
+++ b/net/ipv4/udp_offload.c
@@ -402,6 +402,12 @@ struct sk_buff *udp_gro_receive(struct list_head *head, struct sk_buff *skb,
struct sock *sk;
rcu_read_lock();
+ if (NAPI_GRO_CB(skb)->is_fwd) {
+ pp = call_gro_receive(udp_gro_receive_segment, head, skb);
+ rcu_read_unlock();
+ return pp;
+ }
+
sk = (*lookup)(skb, uh->source, uh->dest);
if (!sk)
goto out_unlock;
@@ -456,7 +462,8 @@ static struct sk_buff *udp4_gro_receive(struct list_head *head,
{
struct udphdr *uh = udp_gro_udphdr(skb);
- if (unlikely(!uh) || !static_branch_unlikely(&udp_encap_needed_key))
+ if (unlikely(!uh) || (!static_branch_unlikely(&udp_encap_needed_key) &&
+ !NAPI_GRO_CB(skb)->is_fwd))
goto flush;
/* Don't bother verifying checksum if we're going to flush anyway. */
@@ -503,7 +510,7 @@ int udp_gro_complete(struct sk_buff *skb, int nhoff,
rcu_read_lock();
sk = (*lookup)(skb, uh->source, uh->dest);
- if (sk && udp_sk(sk)->gro_enabled) {
+ if ((sk && udp_sk(sk)->gro_enabled) || NAPI_GRO_CB(skb)->is_fwd) {
err = udp_gro_complete_segment(skb);
} else if (sk && udp_sk(sk)->gro_complete) {
skb_shinfo(skb)->gso_type = uh->check ? SKB_GSO_UDP_TUNNEL_CSUM
--
2.17.1
^ permalink raw reply related
* [PATCH] net/wan/fsl_ucc_hdlc: add BQL support
From: Mathias Thore @ 2018-10-23 11:48 UTC (permalink / raw)
To: qiang.zhao, linuxppc-dev, netdev, joakim.tjernlund,
david.gounaris
Cc: Mathias Thore
Add byte queue limits support in the fsl_ucc_hdlc driver.
Signed-off-by: Mathias Thore <mathias.thore@infinera.com>
---
Note that this patch is created relative to another patch that was
applied recently: net/wan/fsl_ucc_hdlc: error counters
drivers/net/wan/fsl_ucc_hdlc.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/net/wan/fsl_ucc_hdlc.c b/drivers/net/wan/fsl_ucc_hdlc.c
index 4d6409605207..7a42336c8af8 100644
--- a/drivers/net/wan/fsl_ucc_hdlc.c
+++ b/drivers/net/wan/fsl_ucc_hdlc.c
@@ -391,6 +391,7 @@ static netdev_tx_t ucc_hdlc_tx(struct sk_buff *skb, struct net_device *dev)
dev_kfree_skb(skb);
return -ENOMEM;
}
+ netdev_sent_queue(dev, skb->len);
spin_lock_irqsave(&priv->lock, flags);
/* Start from the next BD that should be filled */
@@ -447,6 +448,8 @@ static int hdlc_tx_done(struct ucc_hdlc_private *priv)
{
/* Start from the next BD that should be filled */
struct net_device *dev = priv->ndev;
+ unsigned int bytes_sent = 0;
+ int howmany = 0;
struct qe_bd *bd; /* BD pointer */
u16 bd_status;
int tx_restart = 0;
@@ -474,6 +477,8 @@ static int hdlc_tx_done(struct ucc_hdlc_private *priv)
skb = priv->tx_skbuff[priv->skb_dirtytx];
if (!skb)
break;
+ howmany++;
+ bytes_sent += skb->len;
dev->stats.tx_packets++;
memset(priv->tx_buffer +
(be32_to_cpu(bd->buf) - priv->dma_tx_addr),
@@ -501,6 +506,7 @@ static int hdlc_tx_done(struct ucc_hdlc_private *priv)
if (tx_restart)
hdlc_tx_restart(priv);
+ netdev_completed_queue(dev, howmany, bytes_sent);
return 0;
}
@@ -721,6 +727,7 @@ static int uhdlc_open(struct net_device *dev)
priv->hdlc_busy = 1;
netif_device_attach(priv->ndev);
napi_enable(&priv->napi);
+ netdev_reset_queue(dev);
netif_start_queue(dev);
hdlc_open(dev);
}
@@ -812,6 +819,7 @@ static int uhdlc_close(struct net_device *dev)
free_irq(priv->ut_info->uf_info.irq, priv);
netif_stop_queue(dev);
+ netdev_reset_queue(dev);
priv->hdlc_busy = 0;
return 0;
--
2.18.1
^ permalink raw reply related
* [PATCH net] Revert "net: simplify sock_poll_wait"
From: Karsten Graul @ 2018-10-23 11:40 UTC (permalink / raw)
To: netdev; +Cc: eric.dumazet, hch, linux-s390
This reverts commit dd979b4df817e9976f18fb6f9d134d6bc4a3c317.
This broke tcp_poll for SMC fallback: An AF_SMC socket establishes an
internal TCP socket for the initial handshake with the remote peer.
Whenever the SMC connection can not be established this TCP socket is
used as a fallback. All socket operations on the SMC socket are then
forwarded to the TCP socket. In case of poll, the file->private_data
pointer references the SMC socket because the TCP socket has no file
assigned. This causes tcp_poll to wait on the wrong socket.
Signed-off-by: Karsten Graul <kgraul@linux.ibm.com>
---
crypto/af_alg.c | 2 +-
include/net/sock.h | 12 +++++++++---
net/atm/common.c | 2 +-
net/caif/caif_socket.c | 2 +-
net/core/datagram.c | 2 +-
net/dccp/proto.c | 2 +-
net/ipv4/tcp.c | 2 +-
net/iucv/af_iucv.c | 2 +-
net/nfc/llcp_sock.c | 2 +-
net/rxrpc/af_rxrpc.c | 2 +-
net/smc/af_smc.c | 2 +-
net/tipc/socket.c | 2 +-
net/unix/af_unix.c | 4 ++--
13 files changed, 22 insertions(+), 16 deletions(-)
diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index b053179e0bc5..17eb09d222ff 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -1071,7 +1071,7 @@ __poll_t af_alg_poll(struct file *file, struct socket *sock,
struct af_alg_ctx *ctx = ask->private;
__poll_t mask;
- sock_poll_wait(file, wait);
+ sock_poll_wait(file, sock, wait);
mask = 0;
if (!ctx->more || ctx->used)
diff --git a/include/net/sock.h b/include/net/sock.h
index 433f45fc2d68..c64a1cff9eb3 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -2057,14 +2057,20 @@ static inline bool skwq_has_sleeper(struct socket_wq *wq)
/**
* sock_poll_wait - place memory barrier behind the poll_wait call.
* @filp: file
+ * @sock: socket to wait on
* @p: poll_table
*
* See the comments in the wq_has_sleeper function.
+ *
+ * Do not derive sock from filp->private_data here. An SMC socket establishes
+ * an internal TCP socket that is used in the fallback case. All socket
+ * operations on the SMC socket are then forwarded to the TCP socket. In case of
+ * poll, the filp->private_data pointer references the SMC socket because the
+ * TCP socket has no file assigned.
*/
-static inline void sock_poll_wait(struct file *filp, poll_table *p)
+static inline void sock_poll_wait(struct file *filp, struct socket *sock,
+ poll_table *p)
{
- struct socket *sock = filp->private_data;
-
if (!poll_does_not_wait(p)) {
poll_wait(filp, &sock->wq->wait, p);
/* We need to be sure we are in sync with the
diff --git a/net/atm/common.c b/net/atm/common.c
index 9f8cb0d2e71e..a38c174fc766 100644
--- a/net/atm/common.c
+++ b/net/atm/common.c
@@ -653,7 +653,7 @@ __poll_t vcc_poll(struct file *file, struct socket *sock, poll_table *wait)
struct atm_vcc *vcc;
__poll_t mask;
- sock_poll_wait(file, wait);
+ sock_poll_wait(file, sock, wait);
mask = 0;
vcc = ATM_SD(sock);
diff --git a/net/caif/caif_socket.c b/net/caif/caif_socket.c
index d18965f3291f..416717c57cd1 100644
--- a/net/caif/caif_socket.c
+++ b/net/caif/caif_socket.c
@@ -941,7 +941,7 @@ static __poll_t caif_poll(struct file *file,
__poll_t mask;
struct caifsock *cf_sk = container_of(sk, struct caifsock, sk);
- sock_poll_wait(file, wait);
+ sock_poll_wait(file, sock, wait);
mask = 0;
/* exceptional events? */
diff --git a/net/core/datagram.c b/net/core/datagram.c
index 9aac0d63d53e..6a034eb538a1 100644
--- a/net/core/datagram.c
+++ b/net/core/datagram.c
@@ -837,7 +837,7 @@ __poll_t datagram_poll(struct file *file, struct socket *sock,
struct sock *sk = sock->sk;
__poll_t mask;
- sock_poll_wait(file, wait);
+ sock_poll_wait(file, sock, wait);
mask = 0;
/* exceptional events? */
diff --git a/net/dccp/proto.c b/net/dccp/proto.c
index 875858c8b059..43733accf58e 100644
--- a/net/dccp/proto.c
+++ b/net/dccp/proto.c
@@ -325,7 +325,7 @@ __poll_t dccp_poll(struct file *file, struct socket *sock,
__poll_t mask;
struct sock *sk = sock->sk;
- sock_poll_wait(file, wait);
+ sock_poll_wait(file, sock, wait);
if (sk->sk_state == DCCP_LISTEN)
return inet_csk_listen_poll(sk);
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 10c6246396cc..bbd07736fb0f 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -507,7 +507,7 @@ __poll_t tcp_poll(struct file *file, struct socket *sock, poll_table *wait)
const struct tcp_sock *tp = tcp_sk(sk);
int state;
- sock_poll_wait(file, wait);
+ sock_poll_wait(file, sock, wait);
state = inet_sk_state_load(sk);
if (state == TCP_LISTEN)
diff --git a/net/iucv/af_iucv.c b/net/iucv/af_iucv.c
index 634150bff156..69057dccece1 100644
--- a/net/iucv/af_iucv.c
+++ b/net/iucv/af_iucv.c
@@ -1504,7 +1504,7 @@ __poll_t iucv_sock_poll(struct file *file, struct socket *sock,
struct sock *sk = sock->sk;
__poll_t mask = 0;
- sock_poll_wait(file, wait);
+ sock_poll_wait(file, sock, wait);
if (sk->sk_state == IUCV_LISTEN)
return iucv_accept_poll(sk);
diff --git a/net/nfc/llcp_sock.c b/net/nfc/llcp_sock.c
index dd4adf8b1167..ae296273ce3d 100644
--- a/net/nfc/llcp_sock.c
+++ b/net/nfc/llcp_sock.c
@@ -556,7 +556,7 @@ static __poll_t llcp_sock_poll(struct file *file, struct socket *sock,
pr_debug("%p\n", sk);
- sock_poll_wait(file, wait);
+ sock_poll_wait(file, sock, wait);
if (sk->sk_state == LLCP_LISTEN)
return llcp_accept_poll(sk);
diff --git a/net/rxrpc/af_rxrpc.c b/net/rxrpc/af_rxrpc.c
index ac44d8afffb1..3c39b8805d01 100644
--- a/net/rxrpc/af_rxrpc.c
+++ b/net/rxrpc/af_rxrpc.c
@@ -741,7 +741,7 @@ static __poll_t rxrpc_poll(struct file *file, struct socket *sock,
struct rxrpc_sock *rx = rxrpc_sk(sk);
__poll_t mask;
- sock_poll_wait(file, wait);
+ sock_poll_wait(file, sock, wait);
mask = 0;
/* the socket is readable if there are any messages waiting on the Rx
diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index b1322607e7f5..63f08b4e51d6 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -1547,7 +1547,7 @@ static __poll_t smc_poll(struct file *file, struct socket *sock,
mask |= EPOLLERR;
} else {
if (sk->sk_state != SMC_CLOSED)
- sock_poll_wait(file, wait);
+ sock_poll_wait(file, sock, wait);
if (sk->sk_err)
mask |= EPOLLERR;
if ((sk->sk_shutdown == SHUTDOWN_MASK) ||
diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index 49810fdff4c5..0bf8ad486c5e 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -715,7 +715,7 @@ static __poll_t tipc_poll(struct file *file, struct socket *sock,
struct tipc_sock *tsk = tipc_sk(sk);
__poll_t revents = 0;
- sock_poll_wait(file, wait);
+ sock_poll_wait(file, sock, wait);
if (sk->sk_shutdown & RCV_SHUTDOWN)
revents |= EPOLLRDHUP | EPOLLIN | EPOLLRDNORM;
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index d1edfa3cad61..c754f3a90a2e 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -2640,7 +2640,7 @@ static __poll_t unix_poll(struct file *file, struct socket *sock, poll_table *wa
struct sock *sk = sock->sk;
__poll_t mask;
- sock_poll_wait(file, wait);
+ sock_poll_wait(file, sock, wait);
mask = 0;
/* exceptional events? */
@@ -2677,7 +2677,7 @@ static __poll_t unix_dgram_poll(struct file *file, struct socket *sock,
unsigned int writable;
__poll_t mask;
- sock_poll_wait(file, wait);
+ sock_poll_wait(file, sock, wait);
mask = 0;
/* exceptional events? */
--
2.18.0.windows.1
^ permalink raw reply related
* Re: [PATCH net v2 0/3] Bugfix for the netsec driver
From: Masahisa Kojima @ 2018-10-23 11:39 UTC (permalink / raw)
To: Ard Biesheuvel, Florian Fainelli
Cc: netdev, Ilias Apalodimas, Jassi Brar, Yoshitoyo Osaki
In-Reply-To: <CAKv+Gu_Y1yDWu4uKvBTB3hkoktLawaYPCqKhezHKxX6b93HMzQ@mail.gmail.com>
Hi Florian, Ard,
Yes, that is my mistake. Thank you very much for pointing out, Ard.
On Tue, 23 Oct 2018 at 20:32, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
> (+ Florian)
>
> On 23 October 2018 at 08:24, <masahisa.kojima@linaro.org> wrote:
> > From: Masahisa Kojima <masahisa.kojima@linaro.org>
> >
> > This patch series include bugfix for the netsec ethernet
> > controller driver, fix the problem in interface down/up.
> >
> > changes in v2:
> > - change the place to perform the PHY power down
> > - use the MACROs defiend in include/uapi/linux/mii.h
> > - update commit comment
> >
> > Masahisa Kojima (3):
> > net: socionext: Stop PHY before resetting netsec
> > net: socionext: Add dummy PHY register read in phy_write()
> > net: socionext: Reset tx queue in ndo_stop
> >
> > drivers/net/ethernet/socionext/netsec.c | 40 ++++++++++++++++++++++++++++-----
> > 1 file changed, 34 insertions(+), 6 deletions(-)
> >
>
> Hello Masahisa,
>
> As a courtesy, please cc people that have commented on your patches on
> subsequent revisions of the series.
^ permalink raw reply
* Re: [RFC PATCH] cgroup, netclassid: add a preemption point to write_classid
From: Tejun Heo @ 2018-10-23 19:58 UTC (permalink / raw)
To: Michal Hocko
Cc: David S. Miller, Johannes Weiner, cgroups, netdev, LKML,
Michal Hocko
In-Reply-To: <20181018085617.28456-1-mhocko@kernel.org>
On Thu, Oct 18, 2018 at 10:56:17AM +0200, Michal Hocko wrote:
> From: Michal Hocko <mhocko@suse.com>
>
> We have seen a customer complaining about soft lockups on !PREEMPT
> kernel config with 4.4 based kernel
...
> If a cgroup has many tasks with many open file descriptors then we would
> end up in a large loop without any rescheduling point throught the
> operation. Add cond_resched once per task.
>
> Signed-off-by: Michal Hocko <mhocko@suse.com>
Applied to cgroup/for-4.20.
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCH net v2 0/3] Bugfix for the netsec driver
From: Ard Biesheuvel @ 2018-10-23 11:32 UTC (permalink / raw)
To: Masahisa Kojima, Florian Fainelli
Cc: <netdev@vger.kernel.org>, Ilias Apalodimas, Jaswinder Singh,
Yoshitoyo Osaki
In-Reply-To: <20181023112428.6785-1-masahisa.kojima@linaro.org>
(+ Florian)
On 23 October 2018 at 08:24, <masahisa.kojima@linaro.org> wrote:
> From: Masahisa Kojima <masahisa.kojima@linaro.org>
>
> This patch series include bugfix for the netsec ethernet
> controller driver, fix the problem in interface down/up.
>
> changes in v2:
> - change the place to perform the PHY power down
> - use the MACROs defiend in include/uapi/linux/mii.h
> - update commit comment
>
> Masahisa Kojima (3):
> net: socionext: Stop PHY before resetting netsec
> net: socionext: Add dummy PHY register read in phy_write()
> net: socionext: Reset tx queue in ndo_stop
>
> drivers/net/ethernet/socionext/netsec.c | 40 ++++++++++++++++++++++++++++-----
> 1 file changed, 34 insertions(+), 6 deletions(-)
>
Hello Masahisa,
As a courtesy, please cc people that have commented on your patches on
subsequent revisions of the series.
^ permalink raw reply
* [PATCH net v2 3/3] net: socionext: Reset tx queue in ndo_stop
From: masahisa.kojima @ 2018-10-23 11:24 UTC (permalink / raw)
To: netdev
Cc: ilias.apalodimas, jaswinder.singh, ard.biesheuvel,
osaki.yoshitoyo, Masahisa Kojima
In-Reply-To: <20181023112428.6785-1-masahisa.kojima@linaro.org>
From: Masahisa Kojima <masahisa.kojima@linaro.org>
We observed that packets and bytes count are not reset
when user performs interface down. Eventually, tx queue is
exhausted and packets will not be sent out.
To avoid this problem, resets tx queue in ndo_stop.
Fixes: 533dd11a12f6 ("net: socionext: Add Synquacer NetSec driver")
Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
Signed-off-by: Yoshitoyo Osaki <osaki.yoshitoyo@socionext.com>
---
changes in v2:
- update commit comment
drivers/net/ethernet/socionext/netsec.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/ethernet/socionext/netsec.c b/drivers/net/ethernet/socionext/netsec.c
index 5c295cc0b8f8..d4da7e017207 100644
--- a/drivers/net/ethernet/socionext/netsec.c
+++ b/drivers/net/ethernet/socionext/netsec.c
@@ -952,6 +952,9 @@ static void netsec_uninit_pkt_dring(struct netsec_priv *priv, int id)
dring->head = 0;
dring->tail = 0;
dring->pkt_cnt = 0;
+
+ if (id == NETSEC_RING_TX)
+ netdev_reset_queue(priv->ndev);
}
static void netsec_free_dring(struct netsec_priv *priv, int id)
--
2.14.2
^ permalink raw reply related
* [PATCH net v2 2/3] net: socionext: Add dummy PHY register read in phy_write()
From: masahisa.kojima @ 2018-10-23 11:24 UTC (permalink / raw)
To: netdev
Cc: ilias.apalodimas, jaswinder.singh, ard.biesheuvel,
osaki.yoshitoyo, Masahisa Kojima
In-Reply-To: <20181023112428.6785-1-masahisa.kojima@linaro.org>
From: Masahisa Kojima <masahisa.kojima@linaro.org>
There is a compatibility issue between RTL8211E implemented
in Developerbox and netsec ethernet controller IP.
Our MDIO controller stops MDC clock right after the write
access, but RTL8211E expects MDC clock must be kept toggling
for several clock cycle with MDIO high before entering
the IDLE state. Without keeping clock after write access,
write access is not correctly handled and register is not
updated.
To meet this requirement, netsec driver needs to issue dummy
read(e.g. read PHYID1(offset 0x2) register) right after write
access, to keep MDC clock.
We think this compatibility issue is a problem specific to
our MDIO controller and RTL8211E.
Fixes: 533dd11a12f6 ("net: socionext: Add Synquacer NetSec driver")
Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
Signed-off-by: Yoshitoyo Osaki <osaki.yoshitoyo@socionext.com>
---
changes in v2:
- use the MACROs defiend in include/uapi/linux/mii.h
drivers/net/ethernet/socionext/netsec.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/socionext/netsec.c b/drivers/net/ethernet/socionext/netsec.c
index 829ed2718b22..5c295cc0b8f8 100644
--- a/drivers/net/ethernet/socionext/netsec.c
+++ b/drivers/net/ethernet/socionext/netsec.c
@@ -432,9 +432,12 @@ static int netsec_mac_update_to_phy_state(struct netsec_priv *priv)
return 0;
}
+static int netsec_phy_read(struct mii_bus *bus, int phy_addr, int reg_addr);
+
static int netsec_phy_write(struct mii_bus *bus,
int phy_addr, int reg, u16 val)
{
+ int status;
struct netsec_priv *priv = bus->priv;
if (netsec_mac_write(priv, GMAC_REG_GDR, val))
@@ -447,8 +450,19 @@ static int netsec_phy_write(struct mii_bus *bus,
GMAC_REG_SHIFT_CR_GAR)))
return -ETIMEDOUT;
- return netsec_mac_wait_while_busy(priv, GMAC_REG_GAR,
- NETSEC_GMAC_GAR_REG_GB);
+ status = netsec_mac_wait_while_busy(priv, GMAC_REG_GAR,
+ NETSEC_GMAC_GAR_REG_GB);
+
+ /* Developerbox implements RTL8211E PHY and there is
+ * a compatibility problem with F_GMAC4.
+ * RTL8211E expects MDC clock must be kept toggling for several
+ * clock cycle with MDIO high before entering the IDLE state.
+ * To meet this requirement, netsec driver needs to issue dummy
+ * read(e.g. read PHYID1(offset 0x2) register) right after write.
+ */
+ netsec_phy_read(bus, phy_addr, MII_PHYSID1);
+
+ return status;
}
static int netsec_phy_read(struct mii_bus *bus, int phy_addr, int reg_addr)
--
2.14.2
^ permalink raw reply related
* [PATCH net v2 1/3] net: socionext: Stop PHY before resetting netsec
From: masahisa.kojima @ 2018-10-23 11:24 UTC (permalink / raw)
To: netdev
Cc: ilias.apalodimas, jaswinder.singh, ard.biesheuvel,
osaki.yoshitoyo, Masahisa Kojima
In-Reply-To: <20181023112428.6785-1-masahisa.kojima@linaro.org>
From: Masahisa Kojima <masahisa.kojima@linaro.org>
In ndo_stop, driver resets the netsec ethernet controller IP.
When the netsec IP is reset, HW running mode turns to NRM mode
and driver has to wait until this mode transition completes.
But mode transition to NRM will not complete if the PHY is
in normal operation state. Netsec IP requires PHY is in
power down state when it is reset.
This modification stops the PHY before resetting netsec.
Together with this modification, phy_addr is stored in netsec_priv
structure because ndev->phydev is not yet ready in ndo_init.
Fixes: 533dd11a12f6 ("net: socionext: Add Synquacer NetSec driver")
Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
Signed-off-by: Yoshitoyo Osaki <osaki.yoshitoyo@socionext.com>
---
changes in v2:
- change the place to perform the PHY power down
- use the MACROs defiend in include/uapi/linux/mii.h
drivers/net/ethernet/socionext/netsec.c | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/socionext/netsec.c b/drivers/net/ethernet/socionext/netsec.c
index 7aa5ebb6766c..829ed2718b22 100644
--- a/drivers/net/ethernet/socionext/netsec.c
+++ b/drivers/net/ethernet/socionext/netsec.c
@@ -274,6 +274,7 @@ struct netsec_priv {
struct clk *clk;
u32 msg_enable;
u32 freq;
+ u32 phy_addr;
bool rx_cksum_offload_flag;
};
@@ -1340,11 +1341,11 @@ static int netsec_netdev_stop(struct net_device *ndev)
netsec_uninit_pkt_dring(priv, NETSEC_RING_TX);
netsec_uninit_pkt_dring(priv, NETSEC_RING_RX);
- ret = netsec_reset_hardware(priv, false);
-
phy_stop(ndev->phydev);
phy_disconnect(ndev->phydev);
+ ret = netsec_reset_hardware(priv, false);
+
pm_runtime_put_sync(priv->dev);
return ret;
@@ -1354,6 +1355,7 @@ static int netsec_netdev_init(struct net_device *ndev)
{
struct netsec_priv *priv = netdev_priv(ndev);
int ret;
+ u16 data;
ret = netsec_alloc_dring(priv, NETSEC_RING_TX);
if (ret)
@@ -1363,6 +1365,11 @@ static int netsec_netdev_init(struct net_device *ndev)
if (ret)
goto err1;
+ /* set phy power down */
+ data = netsec_phy_read(priv->mii_bus, priv->phy_addr, MII_BMCR) |
+ BMCR_PDOWN;
+ netsec_phy_write(priv->mii_bus, priv->phy_addr, MII_BMCR, data);
+
ret = netsec_reset_hardware(priv, true);
if (ret)
goto err2;
@@ -1412,7 +1419,7 @@ static const struct net_device_ops netsec_netdev_ops = {
};
static int netsec_of_probe(struct platform_device *pdev,
- struct netsec_priv *priv)
+ struct netsec_priv *priv, u32 *phy_addr)
{
priv->phy_np = of_parse_phandle(pdev->dev.of_node, "phy-handle", 0);
if (!priv->phy_np) {
@@ -1420,6 +1427,8 @@ static int netsec_of_probe(struct platform_device *pdev,
return -EINVAL;
}
+ *phy_addr = of_mdio_parse_addr(&pdev->dev, priv->phy_np);
+
priv->clk = devm_clk_get(&pdev->dev, NULL); /* get by 'phy_ref_clk' */
if (IS_ERR(priv->clk)) {
dev_err(&pdev->dev, "phy_ref_clk not found\n");
@@ -1620,12 +1629,14 @@ static int netsec_probe(struct platform_device *pdev)
}
if (dev_of_node(&pdev->dev))
- ret = netsec_of_probe(pdev, priv);
+ ret = netsec_of_probe(pdev, priv, &phy_addr);
else
ret = netsec_acpi_probe(pdev, priv, &phy_addr);
if (ret)
goto free_ndev;
+ priv->phy_addr = phy_addr;
+
if (!priv->freq) {
dev_err(&pdev->dev, "missing PHY reference clock frequency\n");
ret = -ENODEV;
--
2.14.2
^ permalink raw reply related
* [PATCH net v2 0/3] Bugfix for the netsec driver
From: masahisa.kojima @ 2018-10-23 11:24 UTC (permalink / raw)
To: netdev
Cc: ilias.apalodimas, jaswinder.singh, ard.biesheuvel,
osaki.yoshitoyo, Masahisa Kojima
From: Masahisa Kojima <masahisa.kojima@linaro.org>
This patch series include bugfix for the netsec ethernet
controller driver, fix the problem in interface down/up.
changes in v2:
- change the place to perform the PHY power down
- use the MACROs defiend in include/uapi/linux/mii.h
- update commit comment
Masahisa Kojima (3):
net: socionext: Stop PHY before resetting netsec
net: socionext: Add dummy PHY register read in phy_write()
net: socionext: Reset tx queue in ndo_stop
drivers/net/ethernet/socionext/netsec.c | 40 ++++++++++++++++++++++++++++-----
1 file changed, 34 insertions(+), 6 deletions(-)
--
2.14.2
^ permalink raw reply
* Re: [PATCH v2] rtlwifi: remove set but not used variable 'radiob_array_table' and 'radiob_arraylen'
From: Joe Perches @ 2018-10-23 19:33 UTC (permalink / raw)
To: zhong jiang, kvalo; +Cc: davem, pkshih, linux-wireless, netdev, linux-kernel
In-Reply-To: <1540283282-9772-1-git-send-email-zhongjiang@huawei.com>
On Tue, 2018-10-23 at 16:28 +0800, zhong jiang wrote:
> radiob_array_table' and 'radiob_arraylen' are not used after setting its value.
> It is safe to remove the unused variable. Meanwhile, radio B radio should be
> removed as well. because it will no longer be referenced.
The patch subject is a bit off and too generic here.
This is specific to rtl8723ae and not rtlwifi so it is
probably better for the subject to be something like:
[PATCH] rtl8723ae: Remove set but not used variables and #defines
> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
> ---
> drivers/net/wireless/realtek/rtlwifi/rtl8723ae/phy.c | 5 +----
> drivers/net/wireless/realtek/rtlwifi/rtl8723ae/table.c | 4 ----
> drivers/net/wireless/realtek/rtlwifi/rtl8723ae/table.h | 2 --
^ permalink raw reply
* Re: [PATCH net-next 3/4] net: phy-c45: Implement reset/suspend/resume callbacks
From: Russell King - ARM Linux @ 2018-10-23 10:58 UTC (permalink / raw)
To: Jose Abreu
Cc: Florian Fainelli, Andrew Lunn, netdev, David S. Miller,
Joao Pinto
In-Reply-To: <d7abcce6-e935-8897-0307-8c5a8afd0990@synopsys.com>
On Tue, Oct 23, 2018 at 11:28:09AM +0100, Jose Abreu wrote:
> On 23-10-2018 11:20, Russell King - ARM Linux wrote:
> > I have no idea what you're proposing there - your patches weren't copied
> > to me.
>
> They just set / unset MDIO_CTRL1_LPOWER bit in PCS. I find that
> without this remote end doesn't detect link is down ...
>
> If it's okay for Generic 10G driver I can submit only this and
> manually reset PHY in stmmac driver so that I don't need to
> implement custom PHY driver ...
> BTW, I just found out currently Generic 10G Driver is broken
> without patch 4/4 of this series [1]
>
> [1] https://patchwork.ozlabs.org/patch/987570/
How is it broken - what are the symptoms?
The generic 10G driver is bound not via the normal bus matching and
phy_bus_match(), but via a manual bind in phy_attach_direct(). This
calls the probe function, which is phy_probe(), which initialises
the supported/advertising to the driver's features (which as you note
are zero.)
However, phy_attach_direct() goes on to call phy_init_hw(), which
calls the config_init() method. The config_init() method initialises
the supported/advertising masks to 10GbaseT. This is (partly) what
I refer to when I say that the generic 10G support is crippled - it
only supports this single speed and media.
So the supported/advertising masks should be forced to only 10GbaseT
at the completion of phy_attach_direct().
The "generic 10G" support doesn't do autonegotiation, configuration
or link mode forcing. It only assumes 10GbaseT is supported, and
only checks for the "link up" bits.
It isn't like the non-10G generic PHY support due to history - it
was added in 2014 by Andy Fleming (see 124059fd53af).
BTW, your patch 1 is wrong as well (introducing phy_update_link()).
You don't take account that a 10G phy may have alternative ways of
reading the link (like 88x3310 does, because it has multiple
instances of AN/PCS/PHYXS at 1k offsets.) All the gen10g_*
functions are legacy functions for the crippled "generic" 10G
support.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
^ permalink raw reply
* [iproute PATCH v2] tc: htb: Print default value in hex
From: Phil Sutter @ 2018-10-23 10:36 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
Value of 'default' is assumed to be hexadecimal when parsing, so
consequently it should be printed in hex as well. This is a regression
introduced when adding JSON output.
As requested, also change JSON output to print the value as hex string.
Fixes: f354fa6aa5ff0 ("tc: jsonify htb qdisc")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
Changes since v1:
- Use print_0xhex().
---
tc/q_htb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tc/q_htb.c b/tc/q_htb.c
index c8b2941d945b7..5fb11d28c5c3a 100644
--- a/tc/q_htb.c
+++ b/tc/q_htb.c
@@ -332,7 +332,7 @@ static int htb_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
if (RTA_PAYLOAD(tb[TCA_HTB_INIT]) < sizeof(*gopt)) return -1;
print_int(PRINT_ANY, "r2q", "r2q %d", gopt->rate2quantum);
- print_uint(PRINT_ANY, "default", " default %u", gopt->defcls);
+ print_0xhex(PRINT_ANY, "default", " default %x", gopt->defcls);
print_uint(PRINT_ANY, "direct_packets_stat",
" direct_packets_stat %u", gopt->direct_pkts);
if (show_details) {
--
2.19.0
^ permalink raw reply related
* Re: [RFC PATCH v2 06/10] udp: cope with UDP GRO packet misdirection
From: Steffen Klassert @ 2018-10-23 10:29 UTC (permalink / raw)
To: Paolo Abeni; +Cc: netdev, Willem de Bruijn
In-Reply-To: <38816b5568eab473cc21e14ad58e51e6c04170f2.camel@redhat.com>
On Mon, Oct 22, 2018 at 02:51:56PM +0200, Paolo Abeni wrote:
> > > +
> > > +static int udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
> > > +{
> > > + struct sk_buff *next, *segs;
> > > + int ret;
> > > +
> > > + if (likely(!udp_unexpected_gso(sk, skb)))
> > > + return udp_queue_rcv_one_skb(sk, skb);
> > > +
> > > + BUILD_BUG_ON(sizeof(struct udp_skb_cb) > SKB_SGO_CB_OFFSET);
> > > + __skb_push(skb, -skb_mac_offset(skb));
> > > + segs = udp_rcv_segment(sk, skb);
> > > + for (skb = segs; skb; skb = next) {
> > > + next = skb->next;
> > > + __skb_pull(skb, skb_transport_offset(skb));
> > > + ret = udp_queue_rcv_one_skb(sk, skb);
> >
> > udp_queue_rcv_one_skb() starts with doing a xfrm4_policy_check().
> > Maybe we can do this on the GSO packet instead of the segments.
> > So far this code is just for handling a corner case, but this might
> > change.
>
> I thought about keeping the policy check here, but then I preferred
> what looked the safest option. Perhaps we can improve with a follow-up?
Fair enough. Let's keep it in mind and do it later.
^ permalink raw reply
* Re: [PATCH net-next 3/4] net: phy-c45: Implement reset/suspend/resume callbacks
From: Jose Abreu @ 2018-10-23 10:28 UTC (permalink / raw)
To: Russell King - ARM Linux, Jose Abreu
Cc: Florian Fainelli, Andrew Lunn, netdev, David S. Miller,
Joao Pinto
In-Reply-To: <20181023102023.GM30658@n2100.armlinux.org.uk>
On 23-10-2018 11:20, Russell King - ARM Linux wrote:
> On Tue, Oct 23, 2018 at 11:17:50AM +0100, Jose Abreu wrote:
>> On 22-10-2018 18:13, Florian Fainelli wrote:
>>> On 10/22/18 8:48 AM, Russell King - ARM Linux wrote:
>>>> On Mon, Oct 22, 2018 at 01:47:48PM +0100, Jose Abreu wrote:
>>>>> Hello,
>>>>>
>>>>> On 22-10-2018 13:28, Andrew Lunn wrote:
>>>>>>> EXPORT_SYMBOL_GPL(gen10g_resume);
>>>>>>> @@ -327,7 +381,7 @@ struct phy_driver genphy_10g_driver = {
>>>>>>> .phy_id = 0xffffffff,
>>>>>>> .phy_id_mask = 0xffffffff,
>>>>>>> .name = "Generic 10G PHY",
>>>>>>> - .soft_reset = gen10g_no_soft_reset,
>>>>>>> + .soft_reset = gen10g_soft_reset,
>>>>>>> .config_init = gen10g_config_init,
>>>>>>> .features = 0,
>>>>>>> .aneg_done = genphy_c45_aneg_done,
>>>>>> Hi Jose
>>>>>>
>>>>>> You need to be careful here. There is a reason this is called
>>>>>> gen10g_no_soft_reset, rather than having an empty
>>>>>> gen10g_soft_reset. Some PHYs break when you do a reset. So adding a
>>>>>> gen10g_soft_reset is fine, but don't change this here, without first
>>>>>> understanding the history, and talking to Russell King.
>>>>> Hmm, the reset function only interacts with standard PCS
>>>>> registers, which should always be available ...
>>>>>
>>>>> >From my tests I need to do at least 1 reset during power-up so in
>>>>> ultimate case I can add a feature quirk or similar.
>>>>>
>>>>> Russell, can you please comment ?
>>>> Setting the reset bit on 88x3310 causes the entire device to become
>>>> completely inaccessible until hardware reset. Therefore, this bit
>>>> must _never_ be set for these devices. That said, we have a separate
>>>> driver for these PHYs, but that will only be used for them if it's
>>>> present in the kernel. If we accidentally fall back to the generic
>>>> driver, then we'll screw the 88x3310 until a full hardware reset.
>>>>
>>>> We also have a bunch of net devices that make use of this crippled
>>>> "generic" 10G support - we don't know whether resetting the PHY
>>>> for those systems will cause a regression - maybe board firmware
>>>> already configured the PHY? I can't say either way on that, except
>>>> that we've had crippled 10G support in PHYLIB for a number of years
>>>> now _with_ users, and adding reset support drastically changes the
>>>> subsystem's behaviour for these users.
>>>>
>>>> I would recommend not touching the generic 10G driver, but instead
>>>> implement your own driver for your PHY to avoid causing regressions.
>>>>
>>> Agreed.
>> What about .suspend / .resume ?
> I have no idea what you're proposing there - your patches weren't copied
> to me.
>
They just set / unset MDIO_CTRL1_LPOWER bit in PCS. I find that
without this remote end doesn't detect link is down ...
If it's okay for Generic 10G driver I can submit only this and
manually reset PHY in stmmac driver so that I don't need to
implement custom PHY driver ...
BTW, I just found out currently Generic 10G Driver is broken
without patch 4/4 of this series [1]
[1] https://patchwork.ozlabs.org/patch/987570/
Thanks and Best Regards,
Jose Miguel Abreu
^ permalink raw reply
* Re: [PATCH net-next 3/4] net: phy-c45: Implement reset/suspend/resume callbacks
From: Russell King - ARM Linux @ 2018-10-23 10:20 UTC (permalink / raw)
To: Jose Abreu
Cc: Florian Fainelli, Andrew Lunn, netdev, David S. Miller,
Joao Pinto
In-Reply-To: <943a6194-001a-d6a0-84ba-b93b728ce64b@synopsys.com>
On Tue, Oct 23, 2018 at 11:17:50AM +0100, Jose Abreu wrote:
> On 22-10-2018 18:13, Florian Fainelli wrote:
> > On 10/22/18 8:48 AM, Russell King - ARM Linux wrote:
> >> On Mon, Oct 22, 2018 at 01:47:48PM +0100, Jose Abreu wrote:
> >>> Hello,
> >>>
> >>> On 22-10-2018 13:28, Andrew Lunn wrote:
> >>>>> EXPORT_SYMBOL_GPL(gen10g_resume);
> >>>>> @@ -327,7 +381,7 @@ struct phy_driver genphy_10g_driver = {
> >>>>> .phy_id = 0xffffffff,
> >>>>> .phy_id_mask = 0xffffffff,
> >>>>> .name = "Generic 10G PHY",
> >>>>> - .soft_reset = gen10g_no_soft_reset,
> >>>>> + .soft_reset = gen10g_soft_reset,
> >>>>> .config_init = gen10g_config_init,
> >>>>> .features = 0,
> >>>>> .aneg_done = genphy_c45_aneg_done,
> >>>> Hi Jose
> >>>>
> >>>> You need to be careful here. There is a reason this is called
> >>>> gen10g_no_soft_reset, rather than having an empty
> >>>> gen10g_soft_reset. Some PHYs break when you do a reset. So adding a
> >>>> gen10g_soft_reset is fine, but don't change this here, without first
> >>>> understanding the history, and talking to Russell King.
> >>> Hmm, the reset function only interacts with standard PCS
> >>> registers, which should always be available ...
> >>>
> >>> >From my tests I need to do at least 1 reset during power-up so in
> >>> ultimate case I can add a feature quirk or similar.
> >>>
> >>> Russell, can you please comment ?
> >> Setting the reset bit on 88x3310 causes the entire device to become
> >> completely inaccessible until hardware reset. Therefore, this bit
> >> must _never_ be set for these devices. That said, we have a separate
> >> driver for these PHYs, but that will only be used for them if it's
> >> present in the kernel. If we accidentally fall back to the generic
> >> driver, then we'll screw the 88x3310 until a full hardware reset.
> >>
> >> We also have a bunch of net devices that make use of this crippled
> >> "generic" 10G support - we don't know whether resetting the PHY
> >> for those systems will cause a regression - maybe board firmware
> >> already configured the PHY? I can't say either way on that, except
> >> that we've had crippled 10G support in PHYLIB for a number of years
> >> now _with_ users, and adding reset support drastically changes the
> >> subsystem's behaviour for these users.
> >>
> >> I would recommend not touching the generic 10G driver, but instead
> >> implement your own driver for your PHY to avoid causing regressions.
> >>
> > Agreed.
>
> What about .suspend / .resume ?
I have no idea what you're proposing there - your patches weren't copied
to me.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
^ permalink raw reply
* Re: [PATCH net-next 1/4] net: phy: Use C45 Helpers when forcing PHY
From: Jose Abreu @ 2018-10-23 10:20 UTC (permalink / raw)
To: Florian Fainelli, Jose Abreu, netdev
Cc: Andrew Lunn, David S. Miller, Joao Pinto
In-Reply-To: <8e1a35a5-de85-e7b3-c9f3-524b3313feaa@gmail.com>
On 22-10-2018 18:11, Florian Fainelli wrote:
> On 10/22/18 3:32 AM, Jose Abreu wrote:
>> If PHY is in force state and we have a C45 phy we need to use the
>> standard C45 helpers and not the C22 ones.
>>
>> Signed-off-by: Jose Abreu <joabreu@synopsys.com>
>> Cc: Andrew Lunn <andrew@lunn.ch>
>> Cc: Florian Fainelli <f.fainelli@gmail.com>
>> Cc: "David S. Miller" <davem@davemloft.net>
>> Cc: Joao Pinto <joao.pinto@synopsys.com>
>> ---
>> drivers/net/phy/phy.c | 2 +-
>> include/linux/phy.h | 8 ++++++++
>> 2 files changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
>> index 1d73ac3309ce..0ff4946e208e 100644
>> --- a/drivers/net/phy/phy.c
>> +++ b/drivers/net/phy/phy.c
>> @@ -995,7 +995,7 @@ void phy_state_machine(struct work_struct *work)
>> }
>> break;
>> case PHY_FORCING:
>> - err = genphy_update_link(phydev);
>> + err = phy_update_link(phydev);
>> if (err)
>> break;
>>
>> diff --git a/include/linux/phy.h b/include/linux/phy.h
>> index 3ea87f774a76..02c2ee8bc05b 100644
>> --- a/include/linux/phy.h
>> +++ b/include/linux/phy.h
>> @@ -1044,6 +1044,14 @@ static inline int phy_read_status(struct phy_device *phydev)
>> return genphy_read_status(phydev);
>> }
>>
>> +static inline int phy_update_link(struct phy_device *phydev)
>> +{
>> + if (phydev->is_c45)
>> + return gen10g_read_status(phydev);
> Should not this be genphy_c45_read_link() for symmetry with
> genphy_update_link() which only updates phydev->link?
Hmmm, genphy_c45_read_link() does not update phydev->link ... I
can create a new gen10g_update_link() that wraps around
genphy_c45_read_link() and updates link ...
Thanks and Best Regards,
Jose Miguel Abreu
^ permalink raw reply
* Re: [PATCH net-next 3/4] net: phy-c45: Implement reset/suspend/resume callbacks
From: Jose Abreu @ 2018-10-23 10:17 UTC (permalink / raw)
To: Florian Fainelli, Russell King - ARM Linux, Jose Abreu
Cc: Andrew Lunn, netdev, David S. Miller, Joao Pinto
In-Reply-To: <f8cd48a0-2cf5-5718-a6a6-1e9824834720@gmail.com>
On 22-10-2018 18:13, Florian Fainelli wrote:
> On 10/22/18 8:48 AM, Russell King - ARM Linux wrote:
>> On Mon, Oct 22, 2018 at 01:47:48PM +0100, Jose Abreu wrote:
>>> Hello,
>>>
>>> On 22-10-2018 13:28, Andrew Lunn wrote:
>>>>> EXPORT_SYMBOL_GPL(gen10g_resume);
>>>>> @@ -327,7 +381,7 @@ struct phy_driver genphy_10g_driver = {
>>>>> .phy_id = 0xffffffff,
>>>>> .phy_id_mask = 0xffffffff,
>>>>> .name = "Generic 10G PHY",
>>>>> - .soft_reset = gen10g_no_soft_reset,
>>>>> + .soft_reset = gen10g_soft_reset,
>>>>> .config_init = gen10g_config_init,
>>>>> .features = 0,
>>>>> .aneg_done = genphy_c45_aneg_done,
>>>> Hi Jose
>>>>
>>>> You need to be careful here. There is a reason this is called
>>>> gen10g_no_soft_reset, rather than having an empty
>>>> gen10g_soft_reset. Some PHYs break when you do a reset. So adding a
>>>> gen10g_soft_reset is fine, but don't change this here, without first
>>>> understanding the history, and talking to Russell King.
>>> Hmm, the reset function only interacts with standard PCS
>>> registers, which should always be available ...
>>>
>>> >From my tests I need to do at least 1 reset during power-up so in
>>> ultimate case I can add a feature quirk or similar.
>>>
>>> Russell, can you please comment ?
>> Setting the reset bit on 88x3310 causes the entire device to become
>> completely inaccessible until hardware reset. Therefore, this bit
>> must _never_ be set for these devices. That said, we have a separate
>> driver for these PHYs, but that will only be used for them if it's
>> present in the kernel. If we accidentally fall back to the generic
>> driver, then we'll screw the 88x3310 until a full hardware reset.
>>
>> We also have a bunch of net devices that make use of this crippled
>> "generic" 10G support - we don't know whether resetting the PHY
>> for those systems will cause a regression - maybe board firmware
>> already configured the PHY? I can't say either way on that, except
>> that we've had crippled 10G support in PHYLIB for a number of years
>> now _with_ users, and adding reset support drastically changes the
>> subsystem's behaviour for these users.
>>
>> I would recommend not touching the generic 10G driver, but instead
>> implement your own driver for your PHY to avoid causing regressions.
>>
> Agreed.
What about .suspend / .resume ?
Thanks and Best Regards,
Jose Miguel Abreu
^ permalink raw reply
* BUG: please report to dccp@vger.kernel.org => prev = 2, last = 2 at net/dccp/ccids/lib/packet_history.c:LINE/tfrc_rx_his
From: syzbot @ 2018-10-23 10:13 UTC (permalink / raw)
To: davem, dccp, garsilva, gerrit, linux-kernel, netdev,
syzkaller-bugs
Hello,
syzbot found the following crash on:
HEAD commit: ca9eb48fe01f Merge tag 'regulator-v5.0' of git://git.kerne..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=1482a939400000
kernel config: https://syzkaller.appspot.com/x/.config?x=963b24abf3f7c2d8
dashboard link: https://syzkaller.appspot.com/bug?extid=e786ba000564d103a6fe
compiler: gcc (GCC) 8.0.1 20180413 (experimental)
Unfortunately, I don't have any reproducer for this crash yet.
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+e786ba000564d103a6fe@syzkaller.appspotmail.com
input: syz0 as /devices/virtual/input/input6
BUG: please report to dccp@vger.kernel.org => prev = 2, last = 2 at
net/dccp/ccids/lib/packet_history.c:425/tfrc_rx_hist_sample_rtt()
CPU: 1 PID: 18 Comm: ksoftirqd/1 Not tainted 4.19.0+ #298
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x1c4/0x2b6 lib/dump_stack.c:113
tfrc_rx_hist_sample_rtt.cold.3+0x54/0x5c
net/dccp/ccids/lib/packet_history.c:422
ccid3_hc_rx_packet_recv+0x5c4/0xeb0 net/dccp/ccids/ccid3.c:767
ccid_hc_rx_packet_recv net/dccp/ccid.h:185 [inline]
dccp_deliver_input_to_ccids+0xf0/0x280 net/dccp/input.c:180
dccp_rcv_established+0x87/0xb0 net/dccp/input.c:378
dccp_v4_do_rcv+0x153/0x180 net/dccp/ipv4.c:656
sk_backlog_rcv include/net/sock.h:931 [inline]
__sk_receive_skb+0x3e5/0xec0 net/core/sock.c:473
dccp_v4_rcv+0x10f9/0x1f58 net/dccp/ipv4.c:877
ip_local_deliver_finish+0x2e9/0xda0 net/ipv4/ip_input.c:215
NF_HOOK include/linux/netfilter.h:289 [inline]
ip_local_deliver+0x1e9/0x750 net/ipv4/ip_input.c:256
dst_input include/net/dst.h:450 [inline]
ip_rcv_finish+0x1f9/0x300 net/ipv4/ip_input.c:415
NF_HOOK include/linux/netfilter.h:289 [inline]
ip_rcv+0xed/0x600 net/ipv4/ip_input.c:524
__netif_receive_skb_one_core+0x14d/0x200 net/core/dev.c:4913
__netif_receive_skb+0x2c/0x1e0 net/core/dev.c:5023
process_backlog+0x218/0x6f0 net/core/dev.c:5829
napi_poll net/core/dev.c:6249 [inline]
net_rx_action+0x7c5/0x1950 net/core/dev.c:6315
__do_softirq+0x30c/0xb03 kernel/softirq.c:292
run_ksoftirqd+0x94/0x100 kernel/softirq.c:653
smpboot_thread_fn+0x68b/0xa00 kernel/smpboot.c:164
kthread+0x35a/0x420 kernel/kthread.c:246
ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:413
net_ratelimit: 18 callbacks suppressed
dccp_close: ABORT with 105978 bytes unread
input: syz0 as /devices/virtual/input/input7
input: syz0 as /devices/virtual/input/input8
dccp_close: ABORT with 52730 bytes unread
input: syz0 as /devices/virtual/input/input9
dccp_close: ABORT with 105978 bytes unread
dccp_close: ABORT with 105978 bytes unread
dccp_close: ABORT with 77306 bytes unread
dccp_close: ABORT with 89594 bytes unread
input: syz0 as /devices/virtual/input/input10
input: syz0 as /devices/virtual/input/input11
input: syz0 as /devices/virtual/input/input12
input: syz0 as /devices/virtual/input/input13
input: syz0 as /devices/virtual/input/input14
input: syz0 as /devices/virtual/input/input15
input: syz0 as /devices/virtual/input/input16
input: syz0 as /devices/virtual/input/input17
input: syz0 as /devices/virtual/input/input18
---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
syzbot.
^ permalink raw reply
* Re: Kernel oops with mlx5 and dual XDP redirect programs
From: Toke Høiland-Jørgensen @ 2018-10-23 10:10 UTC (permalink / raw)
To: Saeed Mahameed, netdev@vger.kernel.org
Cc: Eran Ben Elisha, Tariq Toukan, brouer@redhat.com
In-Reply-To: <15797ad1ccee84dfd47c6f45af155806b4ccc228.camel@mellanox.com>
Saeed Mahameed <saeedm@mellanox.com> writes:
> On Thu, 2018-10-18 at 23:53 +0200, Toke Høiland-Jørgensen wrote:
>> Saeed Mahameed <saeedm@mellanox.com> writes:
>>
>> > I think that the mlx5 driver doesn't know how to tell the other
>> > device
>> > to stop transmitting to it while it is resetting.. Maybe tariq or
>> > Jesper know more about this ?
>> > I will look at this tomorrow after noon and will try to repro...
>>
>> Hi Saeed
>>
>> Did you have a chance to poke at this? :)
>
> HI Toke, yes i have been planing to respond but also i wanted to dig
> more,
>
> so the root cause is very clear.
>
> 1. core 1 is doing tx_dev->ndo_xdp_xmit()
> 2. core 2 is doing tx_dev->xdp_set() //remove xdp program.
Right, it was also my guess that it was related to this interaction.
Thanks for looking into it!
> and the problem is beyond mlx5, since we don't have a way to tell a
> different core/different netdev to stop xmitting, or at least
> synchronize with it.
Hmm, ideally there should be some way for the higher level XDP API to
notice this and abort the call before it even reaches the driver on the
TX side, shouldn't there? At LPC, Jesper and I will be talking about a
proposal for decoupling the ndo_xdp_xmit() resource allocation from
loading and unloading XDP programs, which I guess could be a way to deal
with this as well.
In the meantime...
> I will be waiting for your confirmation that the fix did work.
I tested your patch, and it does indeed fix the crash. However, it also
seems to have the effect that the XDP redirect continues to function
even after removing the XDP program on the target device.
I.e., after the call to ./xdp_fwd -d $TX_IF, I still see packets being
redirected out $TX_IF. Is this intentional?
-Toke
^ permalink raw reply
* [PATCH v3 3/4] net: emac: remove IBM_EMAC_RX_SKB_HEADROOM
From: Christian Lamparter @ 2018-10-23 10:04 UTC (permalink / raw)
To: netdev; +Cc: David S . Miller
In-Reply-To: <e514e6560fd190f4dcfce574ca5ac6a26640c9ff.1540289031.git.chunkeey@gmail.com>
The EMAC driver had a custom IBM_EMAC_RX_SKB_HEADROOM
Kconfig option that reserved additional skb headroom for RX.
This patch removes the option and migrates the code
to use napi_alloc_skb() and netdev_alloc_skb_ip_align()
in its place.
Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
---
drivers/net/ethernet/ibm/emac/Kconfig | 12 ------
drivers/net/ethernet/ibm/emac/core.c | 57 +++++++++++++++++++--------
drivers/net/ethernet/ibm/emac/core.h | 10 ++---
3 files changed, 43 insertions(+), 36 deletions(-)
diff --git a/drivers/net/ethernet/ibm/emac/Kconfig b/drivers/net/ethernet/ibm/emac/Kconfig
index 90d49191beb3..eacf7e141fdc 100644
--- a/drivers/net/ethernet/ibm/emac/Kconfig
+++ b/drivers/net/ethernet/ibm/emac/Kconfig
@@ -28,18 +28,6 @@ config IBM_EMAC_RX_COPY_THRESHOLD
depends on IBM_EMAC
default "256"
-config IBM_EMAC_RX_SKB_HEADROOM
- int "Additional RX skb headroom (bytes)"
- depends on IBM_EMAC
- default "0"
- help
- Additional receive skb headroom. Note, that driver
- will always reserve at least 2 bytes to make IP header
- aligned, so usually there is no need to add any additional
- headroom.
-
- If unsure, set to 0.
-
config IBM_EMAC_DEBUG
bool "Debugging"
depends on IBM_EMAC
diff --git a/drivers/net/ethernet/ibm/emac/core.c b/drivers/net/ethernet/ibm/emac/core.c
index 80aafd7552aa..266b6614125b 100644
--- a/drivers/net/ethernet/ibm/emac/core.c
+++ b/drivers/net/ethernet/ibm/emac/core.c
@@ -1075,7 +1075,9 @@ static int emac_resize_rx_ring(struct emac_instance *dev, int new_mtu)
/* Second pass, allocate new skbs */
for (i = 0; i < NUM_RX_BUFF; ++i) {
- struct sk_buff *skb = alloc_skb(rx_skb_size, GFP_ATOMIC);
+ struct sk_buff *skb;
+
+ skb = netdev_alloc_skb_ip_align(dev->ndev, rx_skb_size);
if (!skb) {
ret = -ENOMEM;
goto oom;
@@ -1084,7 +1086,6 @@ static int emac_resize_rx_ring(struct emac_instance *dev, int new_mtu)
BUG_ON(!dev->rx_skb[i]);
dev_kfree_skb(dev->rx_skb[i]);
- skb_reserve(skb, EMAC_RX_SKB_HEADROOM + 2);
dev->rx_desc[i].data_ptr =
dma_map_single(&dev->ofdev->dev, skb->data - 2, rx_sync_size,
DMA_FROM_DEVICE) + 2;
@@ -1205,20 +1206,18 @@ static void emac_clean_rx_ring(struct emac_instance *dev)
}
}
-static inline int emac_alloc_rx_skb(struct emac_instance *dev, int slot,
- gfp_t flags)
+static inline int
+__emac_prepare_rx_skb(struct sk_buff *skb, struct emac_instance *dev, int slot)
{
- struct sk_buff *skb = alloc_skb(dev->rx_skb_size, flags);
if (unlikely(!skb))
return -ENOMEM;
dev->rx_skb[slot] = skb;
dev->rx_desc[slot].data_len = 0;
- skb_reserve(skb, EMAC_RX_SKB_HEADROOM + 2);
dev->rx_desc[slot].data_ptr =
- dma_map_single(&dev->ofdev->dev, skb->data - 2, dev->rx_sync_size,
- DMA_FROM_DEVICE) + 2;
+ dma_map_single(&dev->ofdev->dev, skb->data - NET_IP_ALIGN,
+ dev->rx_sync_size, DMA_FROM_DEVICE) + NET_IP_ALIGN;
wmb();
dev->rx_desc[slot].ctrl = MAL_RX_CTRL_EMPTY |
(slot == (NUM_RX_BUFF - 1) ? MAL_RX_CTRL_WRAP : 0);
@@ -1226,6 +1225,27 @@ static inline int emac_alloc_rx_skb(struct emac_instance *dev, int slot,
return 0;
}
+static inline int
+emac_alloc_rx_skb(struct emac_instance *dev, int slot)
+{
+ struct sk_buff *skb;
+
+ skb = __netdev_alloc_skb_ip_align(dev->ndev, dev->rx_skb_size,
+ GFP_KERNEL);
+
+ return __emac_prepare_rx_skb(skb, dev, slot);
+}
+
+static inline int
+emac_alloc_rx_skb_napi(struct emac_instance *dev, int slot)
+{
+ struct sk_buff *skb;
+
+ skb = napi_alloc_skb(&dev->mal->napi, dev->rx_skb_size);
+
+ return __emac_prepare_rx_skb(skb, dev, slot);
+}
+
static void emac_print_link_status(struct emac_instance *dev)
{
if (netif_carrier_ok(dev->ndev))
@@ -1256,7 +1276,7 @@ static int emac_open(struct net_device *ndev)
/* Allocate RX ring */
for (i = 0; i < NUM_RX_BUFF; ++i)
- if (emac_alloc_rx_skb(dev, i, GFP_KERNEL)) {
+ if (emac_alloc_rx_skb(dev, i)) {
printk(KERN_ERR "%s: failed to allocate RX ring\n",
ndev->name);
goto oom;
@@ -1779,8 +1799,9 @@ static inline void emac_recycle_rx_skb(struct emac_instance *dev, int slot,
DBG2(dev, "recycle %d %d" NL, slot, len);
if (len)
- dma_map_single(&dev->ofdev->dev, skb->data - 2,
- EMAC_DMA_ALIGN(len + 2), DMA_FROM_DEVICE);
+ dma_map_single(&dev->ofdev->dev, skb->data - NET_IP_ALIGN,
+ SKB_DATA_ALIGN(len + NET_IP_ALIGN),
+ DMA_FROM_DEVICE);
dev->rx_desc[slot].data_len = 0;
wmb();
@@ -1888,16 +1909,18 @@ static int emac_poll_rx(void *param, int budget)
}
if (len && len < EMAC_RX_COPY_THRESH) {
- struct sk_buff *copy_skb =
- alloc_skb(len + EMAC_RX_SKB_HEADROOM + 2, GFP_ATOMIC);
+ struct sk_buff *copy_skb;
+
+ copy_skb = napi_alloc_skb(&dev->mal->napi, len);
if (unlikely(!copy_skb))
goto oom;
- skb_reserve(copy_skb, EMAC_RX_SKB_HEADROOM + 2);
- memcpy(copy_skb->data - 2, skb->data - 2, len + 2);
+ memcpy(copy_skb->data - NET_IP_ALIGN,
+ skb->data - NET_IP_ALIGN,
+ len + NET_IP_ALIGN);
emac_recycle_rx_skb(dev, slot, len);
skb = copy_skb;
- } else if (unlikely(emac_alloc_rx_skb(dev, slot, GFP_ATOMIC)))
+ } else if (unlikely(emac_alloc_rx_skb_napi(dev, slot)))
goto oom;
skb_put(skb, len);
@@ -1918,7 +1941,7 @@ static int emac_poll_rx(void *param, int budget)
sg:
if (ctrl & MAL_RX_CTRL_FIRST) {
BUG_ON(dev->rx_sg_skb);
- if (unlikely(emac_alloc_rx_skb(dev, slot, GFP_ATOMIC))) {
+ if (unlikely(emac_alloc_rx_skb_napi(dev, slot))) {
DBG(dev, "rx OOM %d" NL, slot);
++dev->estats.rx_dropped_oom;
emac_recycle_rx_skb(dev, slot, 0);
diff --git a/drivers/net/ethernet/ibm/emac/core.h b/drivers/net/ethernet/ibm/emac/core.h
index 0bcfe952a3cf..0faeb7c7e958 100644
--- a/drivers/net/ethernet/ibm/emac/core.h
+++ b/drivers/net/ethernet/ibm/emac/core.h
@@ -68,22 +68,18 @@ static inline int emac_rx_size(int mtu)
return mal_rx_size(ETH_DATA_LEN + EMAC_MTU_OVERHEAD);
}
-#define EMAC_DMA_ALIGN(x) ALIGN((x), dma_get_cache_alignment())
-
-#define EMAC_RX_SKB_HEADROOM \
- EMAC_DMA_ALIGN(CONFIG_IBM_EMAC_RX_SKB_HEADROOM)
-
/* Size of RX skb for the given MTU */
static inline int emac_rx_skb_size(int mtu)
{
int size = max(mtu + EMAC_MTU_OVERHEAD, emac_rx_size(mtu));
- return EMAC_DMA_ALIGN(size + 2) + EMAC_RX_SKB_HEADROOM;
+
+ return SKB_DATA_ALIGN(size + NET_SKB_PAD + NET_IP_ALIGN);
}
/* RX DMA sync size */
static inline int emac_rx_sync_size(int mtu)
{
- return EMAC_DMA_ALIGN(emac_rx_size(mtu) + 2);
+ return SKB_DATA_ALIGN(emac_rx_size(mtu) + NET_IP_ALIGN);
}
/* Driver statistcs is split into two parts to make it more cache friendly:
--
2.19.1
^ permalink raw reply related
* [PATCH v3 2/4] net: emac: implement TCP segmentation offload (TSO)
From: Christian Lamparter @ 2018-10-23 10:04 UTC (permalink / raw)
To: netdev; +Cc: David S . Miller
In-Reply-To: <f4acff06dde1a69a3c3f4fdd27014de3a3bd51cb.1540289031.git.chunkeey@gmail.com>
This patch enables TSO(v4) hw feature for emac driver.
As atleast the APM82181's TCP/IP acceleration hardware
controller (TAH) provides TCP segmentation support in
the transmit path.
Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
---
drivers/net/ethernet/ibm/emac/core.c | 112 ++++++++++++++++++++++++++-
drivers/net/ethernet/ibm/emac/core.h | 7 ++
drivers/net/ethernet/ibm/emac/emac.h | 7 ++
drivers/net/ethernet/ibm/emac/tah.c | 22 +++++-
drivers/net/ethernet/ibm/emac/tah.h | 2 +
5 files changed, 148 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/ibm/emac/core.c b/drivers/net/ethernet/ibm/emac/core.c
index be560f9031f4..80aafd7552aa 100644
--- a/drivers/net/ethernet/ibm/emac/core.c
+++ b/drivers/net/ethernet/ibm/emac/core.c
@@ -38,6 +38,9 @@
#include <linux/mii.h>
#include <linux/bitops.h>
#include <linux/if_vlan.h>
+#include <linux/ip.h>
+#include <linux/ipv6.h>
+#include <linux/tcp.h>
#include <linux/workqueue.h>
#include <linux/of.h>
#include <linux/of_address.h>
@@ -1118,6 +1121,32 @@ static int emac_resize_rx_ring(struct emac_instance *dev, int new_mtu)
return ret;
}
+/* Restriction applied for the segmentation size
+ * to use HW segmentation offload feature. the size
+ * of the segment must not be less than 168 bytes for
+ * DIX formatted segments, or 176 bytes for
+ * IEEE formatted segments. However based on actual
+ * tests any MTU less than 416 causes excessive retries
+ * due to TX FIFO underruns.
+ */
+const u32 tah_ss[TAH_NO_SSR] = { 1500, 1344, 1152, 960, 768, 416 };
+
+/* look-up matching segment size for the given mtu */
+static void emac_find_tso_ss_for_mtu(struct emac_instance *dev)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(tah_ss); i++) {
+ if (tah_ss[i] <= dev->ndev->mtu)
+ break;
+ }
+ /* if no matching segment size is found, set the tso_ss_mtu_start
+ * variable anyway. This will cause the emac_tx_tso to skip straight
+ * to the software fallback.
+ */
+ dev->tso_ss_mtu_start = i;
+}
+
/* Process ctx, rtnl_lock semaphore */
static int emac_change_mtu(struct net_device *ndev, int new_mtu)
{
@@ -1134,6 +1163,7 @@ static int emac_change_mtu(struct net_device *ndev, int new_mtu)
if (!ret) {
ndev->mtu = new_mtu;
+ emac_find_tso_ss_for_mtu(dev);
dev->rx_skb_size = emac_rx_skb_size(new_mtu);
dev->rx_sync_size = emac_rx_sync_size(new_mtu);
}
@@ -1410,6 +1440,33 @@ static inline u16 emac_tx_csum(struct emac_instance *dev,
return 0;
}
+static int emac_tx_tso(struct emac_instance *dev, struct sk_buff *skb,
+ u16 *ctrl)
+{
+ if (emac_has_feature(dev, EMAC_FTR_TAH_HAS_TSO) && skb_is_gso(skb) &&
+ !!(skb_shinfo(skb)->gso_type & (SKB_GSO_TCPV4 | SKB_GSO_TCPV6))) {
+ u32 seg_size = 0, i;
+
+ /* Get the MTU */
+ seg_size = skb_shinfo(skb)->gso_size + tcp_hdrlen(skb) +
+ skb_network_header_len(skb);
+
+ for (i = dev->tso_ss_mtu_start; i < ARRAY_SIZE(tah_ss); i++) {
+ if (tah_ss[i] > seg_size)
+ continue;
+
+ *ctrl |= EMAC_TX_CTRL_TAH_SSR(i);
+ return 0;
+ }
+
+ /* none found fall back to software */
+ return -EINVAL;
+ }
+
+ *ctrl |= emac_tx_csum(dev, skb);
+ return 0;
+}
+
static inline netdev_tx_t emac_xmit_finish(struct emac_instance *dev, int len)
{
struct emac_regs __iomem *p = dev->emacp;
@@ -1452,6 +1509,46 @@ static inline u16 emac_tx_vlan(struct emac_instance *dev, struct sk_buff *skb)
return 0;
}
+static netdev_tx_t
+emac_start_xmit_sg(struct sk_buff *skb, struct net_device *ndev);
+
+static int
+emac_sw_tso(struct sk_buff *skb, struct net_device *ndev)
+{
+ struct emac_instance *dev = netdev_priv(ndev);
+ struct sk_buff *segs, *curr;
+ unsigned int i, frag_slots;
+
+ /* make sure to not overflow the tx ring */
+ frag_slots = dev->tx_cnt;
+ for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
+ struct skb_frag_struct *frag = &skb_shinfo(skb)->frags[i];
+
+ frag_slots += mal_tx_chunks(skb_frag_size(frag));
+
+ if (frag_slots >= NUM_TX_BUFF)
+ return -ENOSPC;
+ };
+
+ segs = skb_gso_segment(skb, ndev->features &
+ ~(NETIF_F_TSO | NETIF_F_TSO6));
+ if (IS_ERR_OR_NULL(segs)) {
+ ++dev->estats.tx_dropped;
+ dev_kfree_skb_any(skb);
+ } else {
+ while (segs) {
+ curr = segs;
+ segs = curr->next;
+ curr->next = NULL;
+
+ emac_start_xmit_sg(curr, ndev);
+ }
+ dev_consume_skb_any(skb);
+ }
+
+ return 0;
+}
+
/* Tx lock BH */
static netdev_tx_t emac_start_xmit(struct sk_buff *skb, struct net_device *ndev)
{
@@ -1535,7 +1632,12 @@ emac_start_xmit_sg(struct sk_buff *skb, struct net_device *ndev)
goto stop_queue;
ctrl = EMAC_TX_CTRL_GFCS | EMAC_TX_CTRL_GP | MAL_TX_CTRL_READY |
- emac_tx_csum(dev, skb) | emac_tx_vlan(dev, skb);
+ emac_tx_vlan(dev, skb);
+ if (emac_tx_tso(dev, skb, &ctrl)) {
+ if (emac_sw_tso(skb, ndev))
+ goto stop_queue;
+ }
+
slot = dev->tx_slot;
/* skb data */
@@ -2946,6 +3048,9 @@ static int emac_init_config(struct emac_instance *dev)
if (dev->tah_ph != 0) {
#ifdef CONFIG_IBM_EMAC_TAH
dev->features |= EMAC_FTR_HAS_TAH;
+
+ if (of_device_is_compatible(np, "ibm,emac-apm821xx"))
+ dev->features |= EMAC_FTR_TAH_HAS_TSO;
#else
printk(KERN_ERR "%pOF: TAH support not enabled !\n", np);
return -ENXIO;
@@ -3113,6 +3218,8 @@ static int emac_probe(struct platform_device *ofdev)
}
dev->rx_skb_size = emac_rx_skb_size(ndev->mtu);
dev->rx_sync_size = emac_rx_sync_size(ndev->mtu);
+ ndev->gso_max_segs = NUM_TX_BUFF / 2;
+ emac_find_tso_ss_for_mtu(dev);
/* Get pointers to BD rings */
dev->tx_desc =
@@ -3167,6 +3274,9 @@ static int emac_probe(struct platform_device *ofdev)
if (dev->tah_dev) {
ndev->hw_features = NETIF_F_IP_CSUM | NETIF_F_SG;
+ if (emac_has_feature(dev, EMAC_FTR_TAH_HAS_TSO))
+ ndev->hw_features |= NETIF_F_TSO;
+
if (emac_has_feature(dev, EMAC_FTR_HAS_VLAN_CTAG_TX)) {
ndev->vlan_features |= ndev->hw_features;
ndev->hw_features |= NETIF_F_HW_VLAN_CTAG_TX;
diff --git a/drivers/net/ethernet/ibm/emac/core.h b/drivers/net/ethernet/ibm/emac/core.h
index 8d84d439168c..0bcfe952a3cf 100644
--- a/drivers/net/ethernet/ibm/emac/core.h
+++ b/drivers/net/ethernet/ibm/emac/core.h
@@ -245,6 +245,9 @@ struct emac_instance {
u32 xaht_slots_shift;
u32 xaht_width_shift;
+ /* TAH TSO start index */
+ int tso_ss_mtu_start;
+
/* Descriptor management
*/
struct mal_descriptor *tx_desc;
@@ -336,6 +339,8 @@ struct emac_instance {
#define EMAC_FTR_APM821XX_NO_HALF_DUPLEX 0x00001000
/* EMAC can insert 802.1Q tag */
#define EMAC_FTR_HAS_VLAN_CTAG_TX 0x00002000
+/* TAH can do TCP segmentation offload */
+#define EMAC_FTR_TAH_HAS_TSO 0x00004000
/* Right now, we don't quite handle the always/possible masks on the
* most optimal way as we don't have a way to say something like
@@ -352,6 +357,8 @@ enum {
#endif
#ifdef CONFIG_IBM_EMAC_TAH
EMAC_FTR_HAS_TAH |
+ EMAC_FTR_TAH_HAS_TSO |
+
#endif
#ifdef CONFIG_IBM_EMAC_ZMII
EMAC_FTR_HAS_ZMII |
diff --git a/drivers/net/ethernet/ibm/emac/emac.h b/drivers/net/ethernet/ibm/emac/emac.h
index e2f80cca9bed..833967aceb2f 100644
--- a/drivers/net/ethernet/ibm/emac/emac.h
+++ b/drivers/net/ethernet/ibm/emac/emac.h
@@ -266,6 +266,13 @@ struct emac_regs {
#define EMAC_TX_CTRL_IVT 0x0020
#define EMAC_TX_CTRL_RVT 0x0010
#define EMAC_TX_CTRL_TAH_CSUM 0x000e
+#define EMAC_TX_CTRL_TAH_SSR(idx) (((idx) + 1) << 1)
+#define EMAC_TX_CTRL_TAH_SSR5 0x000c
+#define EMAC_TX_CTRL_TAH_SSR4 0x000a
+#define EMAC_TX_CTRL_TAH_SSR3 0x0008
+#define EMAC_TX_CTRL_TAH_SSR2 0x0006
+#define EMAC_TX_CTRL_TAH_SSR1 0x0004
+#define EMAC_TX_CTRL_TAH_SSR0 0x0002
/* EMAC specific TX descriptor status fields (read access) */
#define EMAC_TX_ST_BFCS 0x0200
diff --git a/drivers/net/ethernet/ibm/emac/tah.c b/drivers/net/ethernet/ibm/emac/tah.c
index 9912456dca48..619c08ee22f7 100644
--- a/drivers/net/ethernet/ibm/emac/tah.c
+++ b/drivers/net/ethernet/ibm/emac/tah.c
@@ -45,6 +45,24 @@ void tah_detach(struct platform_device *ofdev, int channel)
mutex_unlock(&dev->lock);
}
+static void tah_set_ssr(struct platform_device *ofdev)
+{
+ struct tah_instance *dev = dev_get_drvdata(&ofdev->dev);
+ struct tah_regs __iomem *p = dev->base;
+ int i;
+
+ mutex_lock(&dev->lock);
+
+ for (i = 0; i < ARRAY_SIZE(tah_ss); i++) {
+ /* Segment size can be up to 16K, but needs
+ * to be a multiple of 2 bytes
+ */
+ out_be32(&p->ssr0 + i, (tah_ss[i] & 0x3ffc) << 16);
+ }
+
+ mutex_unlock(&dev->lock);
+}
+
void tah_reset(struct platform_device *ofdev)
{
struct tah_instance *dev = platform_get_drvdata(ofdev);
@@ -64,6 +82,8 @@ void tah_reset(struct platform_device *ofdev)
out_be32(&p->mr,
TAH_MR_CVR | TAH_MR_ST_768 | TAH_MR_TFS_10KB | TAH_MR_DTFP |
TAH_MR_DIG);
+
+ tah_set_ssr(ofdev);
}
int tah_get_regs_len(struct platform_device *ofdev)
@@ -118,7 +138,7 @@ static int tah_probe(struct platform_device *ofdev)
platform_set_drvdata(ofdev, dev);
- /* Initialize TAH and enable IPv4 checksum verification, no TSO yet */
+ /* Initialize TAH and enable IPv4 checksum verification */
tah_reset(ofdev);
printk(KERN_INFO "TAH %pOF initialized\n", ofdev->dev.of_node);
diff --git a/drivers/net/ethernet/ibm/emac/tah.h b/drivers/net/ethernet/ibm/emac/tah.h
index 4d5f336f07b3..2cb0629f30e2 100644
--- a/drivers/net/ethernet/ibm/emac/tah.h
+++ b/drivers/net/ethernet/ibm/emac/tah.h
@@ -36,6 +36,8 @@ struct tah_regs {
u32 tsr;
};
+#define TAH_NO_SSR 6
+extern const u32 tah_ss[TAH_NO_SSR];
/* TAH device */
struct tah_instance {
--
2.19.1
^ permalink raw reply related
* [PATCH v3 1/4] net: emac: implement 802.1Q VLAN TX tagging support
From: Christian Lamparter @ 2018-10-23 10:04 UTC (permalink / raw)
To: netdev; +Cc: David S . Miller
As per' APM82181 Embedded Processor User Manual 26.1 EMAC Features:
VLAN:
- Support for VLAN tag ID in compliance with IEEE 802.3ac.
- VLAN tag insertion or replacement for transmit packets
This patch completes the missing code for the VLAN tx tagging
support, as the the EMAC_MR1_VLE was already enabled.
Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
---
drivers/net/ethernet/ibm/emac/core.c | 32 ++++++++++++++++++++++++----
drivers/net/ethernet/ibm/emac/core.h | 6 +++++-
2 files changed, 33 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/ibm/emac/core.c b/drivers/net/ethernet/ibm/emac/core.c
index 760b2ad8e295..be560f9031f4 100644
--- a/drivers/net/ethernet/ibm/emac/core.c
+++ b/drivers/net/ethernet/ibm/emac/core.c
@@ -37,6 +37,7 @@
#include <linux/ethtool.h>
#include <linux/mii.h>
#include <linux/bitops.h>
+#include <linux/if_vlan.h>
#include <linux/workqueue.h>
#include <linux/of.h>
#include <linux/of_address.h>
@@ -674,7 +675,7 @@ static int emac_configure(struct emac_instance *dev)
ndev->dev_addr[5]);
/* VLAN Tag Protocol ID */
- out_be32(&p->vtpid, 0x8100);
+ out_be32(&p->vtpid, ETH_P_8021Q);
/* Receive mode register */
r = emac_iff2rmr(ndev);
@@ -1435,6 +1436,22 @@ static inline netdev_tx_t emac_xmit_finish(struct emac_instance *dev, int len)
return NETDEV_TX_OK;
}
+static inline u16 emac_tx_vlan(struct emac_instance *dev, struct sk_buff *skb)
+{
+ /* Handle VLAN TPID and TCI insert if this is a VLAN skb */
+ if (emac_has_feature(dev, EMAC_FTR_HAS_VLAN_CTAG_TX) &&
+ skb_vlan_tag_present(skb)) {
+ struct emac_regs __iomem *p = dev->emacp;
+
+ /* update the VLAN TCI */
+ out_be32(&p->vtci, (u32)skb_vlan_tag_get(skb));
+
+ /* Insert VLAN tag */
+ return EMAC_TX_CTRL_IVT;
+ }
+ return 0;
+}
+
/* Tx lock BH */
static netdev_tx_t emac_start_xmit(struct sk_buff *skb, struct net_device *ndev)
{
@@ -1443,7 +1460,7 @@ static netdev_tx_t emac_start_xmit(struct sk_buff *skb, struct net_device *ndev)
int slot;
u16 ctrl = EMAC_TX_CTRL_GFCS | EMAC_TX_CTRL_GP | MAL_TX_CTRL_READY |
- MAL_TX_CTRL_LAST | emac_tx_csum(dev, skb);
+ MAL_TX_CTRL_LAST | emac_tx_csum(dev, skb) | emac_tx_vlan(dev, skb);
slot = dev->tx_slot++;
if (dev->tx_slot == NUM_TX_BUFF) {
@@ -1518,7 +1535,7 @@ emac_start_xmit_sg(struct sk_buff *skb, struct net_device *ndev)
goto stop_queue;
ctrl = EMAC_TX_CTRL_GFCS | EMAC_TX_CTRL_GP | MAL_TX_CTRL_READY |
- emac_tx_csum(dev, skb);
+ emac_tx_csum(dev, skb) | emac_tx_vlan(dev, skb);
slot = dev->tx_slot;
/* skb data */
@@ -2891,7 +2908,8 @@ static int emac_init_config(struct emac_instance *dev)
if (of_device_is_compatible(np, "ibm,emac-apm821xx")) {
dev->features |= (EMAC_APM821XX_REQ_JUMBO_FRAME_SIZE |
EMAC_FTR_APM821XX_NO_HALF_DUPLEX |
- EMAC_FTR_460EX_PHY_CLK_FIX);
+ EMAC_FTR_460EX_PHY_CLK_FIX |
+ EMAC_FTR_HAS_VLAN_CTAG_TX);
}
} else if (of_device_is_compatible(np, "ibm,emac4")) {
dev->features |= EMAC_FTR_EMAC4;
@@ -3148,6 +3166,12 @@ static int emac_probe(struct platform_device *ofdev)
if (dev->tah_dev) {
ndev->hw_features = NETIF_F_IP_CSUM | NETIF_F_SG;
+
+ if (emac_has_feature(dev, EMAC_FTR_HAS_VLAN_CTAG_TX)) {
+ ndev->vlan_features |= ndev->hw_features;
+ ndev->hw_features |= NETIF_F_HW_VLAN_CTAG_TX;
+ }
+
ndev->features |= ndev->hw_features | NETIF_F_RXCSUM;
}
ndev->watchdog_timeo = 5 * HZ;
diff --git a/drivers/net/ethernet/ibm/emac/core.h b/drivers/net/ethernet/ibm/emac/core.h
index 84caa4a3fc52..8d84d439168c 100644
--- a/drivers/net/ethernet/ibm/emac/core.h
+++ b/drivers/net/ethernet/ibm/emac/core.h
@@ -334,6 +334,8 @@ struct emac_instance {
* APM821xx does not support Half Duplex mode
*/
#define EMAC_FTR_APM821XX_NO_HALF_DUPLEX 0x00001000
+/* EMAC can insert 802.1Q tag */
+#define EMAC_FTR_HAS_VLAN_CTAG_TX 0x00002000
/* Right now, we don't quite handle the always/possible masks on the
* most optimal way as we don't have a way to say something like
@@ -363,7 +365,9 @@ enum {
EMAC_FTR_460EX_PHY_CLK_FIX |
EMAC_FTR_440EP_PHY_CLK_FIX |
EMAC_APM821XX_REQ_JUMBO_FRAME_SIZE |
- EMAC_FTR_APM821XX_NO_HALF_DUPLEX,
+ EMAC_FTR_APM821XX_NO_HALF_DUPLEX |
+ EMAC_FTR_HAS_VLAN_CTAG_TX |
+ 0,
};
static inline int emac_has_feature(struct emac_instance *dev,
--
2.19.1
^ 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