* Re: [PATCH v3 net-next] Introduce a sysctl that modifies the value of PROT_SOCK.
From: David Miller @ 2017-01-24 17:11 UTC (permalink / raw)
To: kjlx; +Cc: eric.dumazet, stephen, netdev
In-Reply-To: <20170121014911.GB2598@templeofstupid.com>
From: Krister Johansen <kjlx@templeofstupid.com>
Date: Fri, 20 Jan 2017 17:49:11 -0800
> Add net.ipv4.ip_unprivileged_port_start, which is a per namespace sysctl
> that denotes the first unprivileged inet port in the namespace. To
> disable all privileged ports set this to zero. It also checks for
> overlap with the local port range. The privileged and local range may
> not overlap.
>
> The use case for this change is to allow containerized processes to bind
> to priviliged ports, but prevent them from ever being allowed to modify
> their container's network configuration. The latter is accomplished by
> ensuring that the network namespace is not a child of the user
> namespace. This modification was needed to allow the container manager
> to disable a namespace's priviliged port restrictions without exposing
> control of the network namespace to processes in the user namespace.
>
> Signed-off-by: Krister Johansen <kjlx@templeofstupid.com>
Applied, thanks.
^ permalink raw reply
* RE: [for-next 07/10] IB/mlx5: Use blue flame register allocator in mlx5_ib
From: Eli Cohen @ 2017-01-24 17:16 UTC (permalink / raw)
To: Doug Ledford, Leon Romanovsky, David Miller
Cc: eli@dev.mellanox.co.il, Saeed Mahameed, netdev@vger.kernel.org,
linux-rdma@vger.kernel.org, Matan Barak
In-Reply-To: <1485276271.43764.31.camel@redhat.com>
It is in Dave's tree:
commit 5fe9dec0d045437e48f112b8fa705197bd7bc3c0
Author: Eli Cohen <eli@mellanox.com>
Date: Tue Jan 3 23:55:25 2017 +0200
IB/mlx5: Use blue flame register allocator in mlx5_ib
Make use of the blue flame registers allocator at mlx5_ib. Since blue
flame was not really supported we remove all the code that is related to
blue flame and we let all consumers to use the same blue flame register.
Once blue flame is supported we will add the code. As part of this patch
we also move the definition of struct mlx5_bf to mlx5_ib.h as it is only
used by mlx5_ib.
Signed-off-by: Eli Cohen <eli@mellanox.com>
Reviewed-by: Matan Barak <matanb@mellanox.com>
Signed-off-by: Leon Romanovsky <leon@kernel.org>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
-----Original Message-----
From: Doug Ledford [mailto:dledford@redhat.com]
Sent: Tuesday, January 24, 2017 10:45 AM
To: Leon Romanovsky <leonro@mellanox.com>; David Miller <davem@redhat.com>
Cc: eli@dev.mellanox.co.il; Saeed Mahameed <saeedm@mellanox.com>; netdev@vger.kernel.org; linux-rdma@vger.kernel.org; Eli Cohen <eli@mellanox.com>; Matan Barak <matanb@mellanox.com>
Subject: Re: [for-next 07/10] IB/mlx5: Use blue flame register allocator in mlx5_ib
On Sun, 2017-01-08 at 08:22 +0200, Leon Romanovsky wrote:
> On Fri, Jan 06, 2017 at 11:11:31AM -0500, David Miller wrote:
> >
> > From: Leon Romanovsky <leonro@mellanox.com>
> > Date: Fri, 6 Jan 2017 08:06:09 +0200
> >
> > >
> > > On Thu, Jan 05, 2017 at 03:07:31PM -0500, David Miller wrote:
> > > >
> > > > From: Eli Cohen <eli@dev.mellanox.co.il>
> > > > Date: Thu, 5 Jan 2017 14:03:18 -0600
> > > >
> > > > >
> > > > > If necessary I can make sure it builds on 32 bits as well.
> > > >
> > > > Please do.
> > >
> > > Dave,
> > >
> > > I'm failing to understand the benefits of building mlx5 on 32
> > > bits, and see only disadvantages:
> > > * It is actual dead code without test coverage.
> > > * It misleads reviewers/customers by seeing code for 32 bits.
> > > * It adds compilation time for 32 bits platforms and "punishes"
> > > them
> > > for not relevant for them driver.
> > >
> > > Why do you call removing all that as a "regression"?
> >
> > We have this thing called "CONFIG_COMPILE_TEST", it has tons of
> > value, perhaps you've seen it before?
>
> Thanks David,
> I see your point.
What's the status update on this. Unless I'm missing it in my rdma mailing list (which is always possible if I scan subjects too fast), I don't see a v2 of this pull request, but I also don't see that David's concerns have been addressed, nor that he has pulled this.
--
Doug Ledford <dledford@redhat.com>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD
^ permalink raw reply
* [PATCH 2/5] gtp: clear DF bit on GTP packet tx
From: Andreas Schultz @ 2017-01-24 17:23 UTC (permalink / raw)
To: Pablo Neira; +Cc: netdev, Lionel Gauthier, openbsc, Harald Welte
In-Reply-To: <20170124172402.12096-1-aschultz@tpip.net>
3GPP TS 29.281 and 3GPP TS 29.060 imply that GTP-U packets should be
sent with the DF bit cleared. For example 3GPP TS 29.060, Release 8,
Section 13.2.2:
> Backbone router: Any router in the backbone may fragment the GTP
> packet if needed, according to IPv4.
Signed-off-by: Andreas Schultz <aschultz@tpip.net>
---
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 7580ccc..1df54d6 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -612,7 +612,7 @@ static netdev_tx_t gtp_dev_xmit(struct sk_buff *skb, struct net_device *dev)
pktinfo.fl4.saddr, pktinfo.fl4.daddr,
pktinfo.iph->tos,
ip4_dst_hoplimit(&pktinfo.rt->dst),
- htons(IP_DF),
+ 0,
pktinfo.gtph_port, pktinfo.gtph_port,
true, false);
break;
--
2.10.2
^ permalink raw reply related
* [PATCH 0/5] simple gtp improvements
From: Andreas Schultz @ 2017-01-24 17:23 UTC (permalink / raw)
To: Pablo Neira; +Cc: netdev, Lionel Gauthier, openbsc, Harald Welte
As requested are here the simple and most obvious changes from
the larger GTP changeset. I'll break down the rest and send them
out separately.
The module alias addition and the rcu_lock removal are just small
convenience changes.
The other changes are needed to correctly implement one of the
3GPP GW functions, userspace needs to see invalid T-PDU's in
order to generate the proper error messages, GTP fragmentation
rules need the cleared DF bit.
Andreas
^ permalink raw reply
* [PATCH 1/5] gtp: add genl family modules alias
From: Andreas Schultz @ 2017-01-24 17:23 UTC (permalink / raw)
To: Pablo Neira; +Cc: netdev, Lionel Gauthier, openbsc, Harald Welte
In-Reply-To: <20170124172402.12096-1-aschultz@tpip.net>
Auto-load the module when userspace asks for the gtp netlink
family.
Signed-off-by: Andreas Schultz <aschultz@tpip.net>
---
drivers/net/gtp.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index 8b6810b..7580ccc 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -1376,3 +1376,4 @@ MODULE_LICENSE("GPL");
MODULE_AUTHOR("Harald Welte <hwelte@sysmocom.de>");
MODULE_DESCRIPTION("Interface driver for GTP encapsulated traffic");
MODULE_ALIAS_RTNL_LINK("gtp");
+MODULE_ALIAS_GENL_FAMILY("gtp");
--
2.10.2
^ permalink raw reply related
* [PATCH 4/5] gtp: remove unnecessary rcu_read_lock
From: Andreas Schultz @ 2017-01-24 17:24 UTC (permalink / raw)
To: Pablo Neira; +Cc: netdev, Lionel Gauthier, openbsc, Harald Welte
In-Reply-To: <20170124172402.12096-1-aschultz@tpip.net>
The rcu read lock is hold by default in the ip input path. There
is no need to hold it twice in the socket recv decapsulate code path.
Signed-off-by: Andreas Schultz <aschultz@tpip.net>
---
drivers/net/gtp.c | 24 ++++--------------------
1 file changed, 4 insertions(+), 20 deletions(-)
diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index 72dd1ba..912721e 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -183,7 +183,6 @@ static int gtp0_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb,
sizeof(struct gtp0_header);
struct gtp0_header *gtp0;
struct pdp_ctx *pctx;
- int ret = 0;
if (!pskb_may_pull(skb, hdrlen))
return -1;
@@ -196,26 +195,19 @@ static int gtp0_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb,
if (gtp0->type != GTP_TPDU)
return 1;
- rcu_read_lock();
pctx = gtp0_pdp_find(gtp, be64_to_cpu(gtp0->tid));
if (!pctx) {
netdev_dbg(gtp->dev, "No PDP ctx to decap skb=%p\n", skb);
- ret = -1;
- goto out_rcu;
+ return -1;
}
if (!gtp_check_src_ms(skb, pctx, hdrlen)) {
netdev_dbg(gtp->dev, "No PDP ctx for this MS\n");
- ret = -1;
- goto out_rcu;
+ return -1;
}
- rcu_read_unlock();
/* Get rid of the GTP + UDP headers. */
return iptunnel_pull_header(skb, hdrlen, skb->protocol, xnet);
-out_rcu:
- rcu_read_unlock();
- return ret;
}
static int gtp1u_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb,
@@ -225,7 +217,6 @@ static int gtp1u_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb,
sizeof(struct gtp1_header);
struct gtp1_header *gtp1;
struct pdp_ctx *pctx;
- int ret = 0;
if (!pskb_may_pull(skb, hdrlen))
return -1;
@@ -253,26 +244,19 @@ static int gtp1u_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb,
gtp1 = (struct gtp1_header *)(skb->data + sizeof(struct udphdr));
- rcu_read_lock();
pctx = gtp1_pdp_find(gtp, ntohl(gtp1->tid));
if (!pctx) {
netdev_dbg(gtp->dev, "No PDP ctx to decap skb=%p\n", skb);
- ret = -1;
- goto out_rcu;
+ return -1;
}
if (!gtp_check_src_ms(skb, pctx, hdrlen)) {
netdev_dbg(gtp->dev, "No PDP ctx for this MS\n");
- ret = -1;
- goto out_rcu;
+ return -1;
}
- rcu_read_unlock();
/* Get rid of the GTP + UDP headers. */
return iptunnel_pull_header(skb, hdrlen, skb->protocol, xnet);
-out_rcu:
- rcu_read_unlock();
- return ret;
}
static void gtp_encap_disable(struct gtp_dev *gtp)
--
2.10.2
^ permalink raw reply related
* [PATCH 3/5] gtp: fix cross netns recv on gtp socket
From: Andreas Schultz @ 2017-01-24 17:24 UTC (permalink / raw)
To: Pablo Neira; +Cc: netdev, Lionel Gauthier, openbsc, Harald Welte
In-Reply-To: <20170124172402.12096-1-aschultz@tpip.net>
The use of the passed through netlink src_net to check for a
cross netns operation was wrong. Using the GTP socket and the
GTP netdevice is always correct (even if the netdev has been
moved to new netns after link creation).
Remove the now obsolete net field from gtp_dev.
Signed-off-by: Andreas Schultz <aschultz@tpip.net>
---
drivers/net/gtp.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index 1df54d6..72dd1ba 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -69,7 +69,6 @@ struct gtp_dev {
struct socket *sock0;
struct socket *sock1u;
- struct net *net;
struct net_device *dev;
unsigned int hash_size;
@@ -316,7 +315,7 @@ static int gtp_encap_recv(struct sock *sk, struct sk_buff *skb)
netdev_dbg(gtp->dev, "encap_recv sk=%p\n", sk);
- xnet = !net_eq(gtp->net, dev_net(gtp->dev));
+ xnet = !net_eq(sock_net(sk), dev_net(gtp->dev));
switch (udp_sk(sk)->encap_type) {
case UDP_ENCAP_GTP0:
@@ -658,7 +657,7 @@ static void gtp_link_setup(struct net_device *dev)
static int gtp_hashtable_new(struct gtp_dev *gtp, int hsize);
static void gtp_hashtable_free(struct gtp_dev *gtp);
static int gtp_encap_enable(struct net_device *dev, struct gtp_dev *gtp,
- int fd_gtp0, int fd_gtp1, struct net *src_net);
+ int fd_gtp0, int fd_gtp1);
static int gtp_newlink(struct net *src_net, struct net_device *dev,
struct nlattr *tb[], struct nlattr *data[])
@@ -858,7 +857,6 @@ static int gtp_encap_enable(struct net_device *dev, struct gtp_dev *gtp,
gtp->sock0 = sock0;
gtp->sock1u = sock1u;
- gtp->net = src_net;
tuncfg.sk_user_data = gtp;
tuncfg.encap_rcv = gtp_encap_recv;
--
2.10.2
^ permalink raw reply related
* [PATCH 5/5] gtp: let userspace handle packets for invalid tunnels
From: Andreas Schultz @ 2017-01-24 17:24 UTC (permalink / raw)
To: Pablo Neira; +Cc: netdev, Lionel Gauthier, openbsc, Harald Welte
In-Reply-To: <20170124172402.12096-1-aschultz@tpip.net>
enable userspace to send error replies for invalid tunnels
Signed-off-by: Andreas Schultz <aschultz@tpip.net>
---
drivers/net/gtp.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index 912721e..c607333 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -198,12 +198,12 @@ static int gtp0_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb,
pctx = gtp0_pdp_find(gtp, be64_to_cpu(gtp0->tid));
if (!pctx) {
netdev_dbg(gtp->dev, "No PDP ctx to decap skb=%p\n", skb);
- return -1;
+ return 1;
}
if (!gtp_check_src_ms(skb, pctx, hdrlen)) {
netdev_dbg(gtp->dev, "No PDP ctx for this MS\n");
- return -1;
+ return 1;
}
/* Get rid of the GTP + UDP headers. */
@@ -247,12 +247,12 @@ static int gtp1u_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb,
pctx = gtp1_pdp_find(gtp, ntohl(gtp1->tid));
if (!pctx) {
netdev_dbg(gtp->dev, "No PDP ctx to decap skb=%p\n", skb);
- return -1;
+ return 1;
}
if (!gtp_check_src_ms(skb, pctx, hdrlen)) {
netdev_dbg(gtp->dev, "No PDP ctx for this MS\n");
- return -1;
+ return 1;
}
/* Get rid of the GTP + UDP headers. */
--
2.10.2
^ permalink raw reply related
* Re: [PATCH net-next 3/3] net/tcp-fastopen: Add new API support
From: Yuchung Cheng @ 2017-01-24 17:26 UTC (permalink / raw)
To: Willy Tarreau; +Cc: Wei Wang, netdev, David Miller, Eric Dumazet, Wei Wang
In-Reply-To: <20170124073043.GA21590@1wt.eu>
On Mon, Jan 23, 2017 at 11:30 PM, Willy Tarreau <w@1wt.eu> wrote:
>
> On Mon, Jan 23, 2017 at 10:59:22AM -0800, Wei Wang wrote:
> > This patch adds a new socket option, TCP_FASTOPEN_CONNECT, as an
> > alternative way to perform Fast Open on the active side (client).
>
> Wei, I think that nothing prevents from reusin the original TCP_FASTOPEN
> sockopt instead of adding a new one. The original one does this :
>
> case TCP_FASTOPEN:
> if (val >= 0 && ((1 << sk->sk_state) & (TCPF_CLOSE |
> TCPF_LISTEN))) {
> tcp_fastopen_init_key_once(true);
>
> fastopen_queue_tune(sk, val);
> } else {
> err = -EINVAL;
> }
> break;
>
> and your new option does this :
>
> case TCP_FASTOPEN_CONNECT:
> if (val > 1 || val < 0) {
> err = -EINVAL;
> } else if (sysctl_tcp_fastopen & TFO_CLIENT_ENABLE) {
> if (sk->sk_state == TCP_CLOSE)
> tp->fastopen_connect = val;
> else
> err = -EINVAL;
> } else {
> err = -EOPNOTSUPP;
> }
> break;
>
> Now if we compare :
> - the value ranges are the same (0,1)
> - tcp_fastopen_init_key_once() only performs an initialization once
> - fastopen_queue_tune() only sets sk->max_qlen based on the backlog,
> this has no effect on an outgoing connection ;
> - tp->fastopen_connect can be applied to a listening socket without
> side effect.
>
> Thus I think we can merge them this way :
>
> case TCP_FASTOPEN:
> if (val >= 0) {
> if ((sysctl_tcp_fastopen & TFO_CLIENT_ENABLE) &&
> (sk->sk_state == TCP_CLOSE)
> tp->fastopen_connect = val;
>
> if ((1 << sk->sk_state) & (TCPF_CLOSE | TCPF_LISTEN))) {
> tcp_fastopen_init_key_once(true);
> fastopen_queue_tune(sk, val);
> }
> } else {
> err = -EINVAL;
> }
> break;
>
> And for the userland, the API is even simpler because we can use the
> same TCP_FASTOPEN sockopt regardless of the socket direction. Also,
> I don't know if TCP_FASTOPEN is supported on simultaneous connect,
> but at least if it works it would be easier to understand this way.
It supports partially (i.e. send SYN data but not accept simul.
SYN-data crossing).
Here is the snippet of packetdrill test we use internally:
`sysctl net.ipv4.tcp_timestamps=0`
// Cache warmup: send a Fast Open cookie request
0.000 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
0.000 fcntl(3, F_SETFL, O_RDWR|O_NONBLOCK) = 0
0.000 sendto(3, ..., 0, MSG_FASTOPEN, ..., ...) = -1 EINPROGRESS
(Operation is now in progress)
0.000 > S 0:0(0) <mss 1460,nop,nop,sackOK,nop,wscale 8,FO,nop,nop>
0.010 < S. 123:123(0) ack 1 win 14600 <mss
1460,nop,nop,sackOK,nop,wscale 6,FO abcd1234,nop,nop>
0.010 > . 1:1(0) ack 1
0.020 close(3) = 0
0.020 > F. 1:1(0) ack 1
0.030 < F. 1:1(0) ack 2 win 92
0.030 > . 2:2(0) ack 2
//
// Test: simulatenous fast open
//
+.010 socket(..., SOCK_STREAM, IPPROTO_TCP) = 4
+.000 fcntl(4, F_SETFL, O_RDWR|O_NONBLOCK) = 0
+.000 sendto(4, ..., 1000, MSG_FASTOPEN, ..., ...) = 1000
+.000 > S 0:1000(1000) <mss 1460,nop,nop,sackOK,nop,wscale 8,FO
abcd1234,nop,nop>
// Simul. SYN-data crossing: we don't support that yet so ack only remote ISN
+.005 < S 1234:1734(500) win 14600 <mss 1040,nop,nop,sackOK,nop,wscale
6,FO 87654321,nop,nop>
+.000 > S. 0:0(0) ack 1235 <mss 1460,nop,nop,sackOK,nop,wscale 8>
// SYN data is never retried.
+.045 < S. 1234:1234(0) ack 1001 win 14600 <mss
940,nop,nop,sackOK,nop,wscale 6,FO 12345678,nop,nop>
+.000 > . 1001:1001(0) ack 1
// The other end retries
+.100 < P. 1:501(500) ack 1000 win 257
+.000 > . 1001:1001(0) ack 501
+.000 read(4, ..., 4096) = 500
+.000 close(4) = 0
+.000 > F. 1001:1001(0) ack 501
+.050 < F. 501:501(0) ack 1002 win 257
+.000 > . 1002:1002(0) ack 502
>
> Do you think there's a compelling reason for adding a new option or
> are you interested in a small patch to perform the change above ?
I like the proposal especially other stack also uses TCP_FASTOPEN
https://msdn.microsoft.com/en-us/library/windows/desktop/ms738596(v=vs.85).aspx
>
> Regards,
> Willy
^ permalink raw reply
* Re: [PATCH net] net: reset ct before calling ndo_start_xmit
From: Stephen Hemminger @ 2017-01-24 17:37 UTC (permalink / raw)
To: Paolo Abeni
Cc: netdev, David S. Miller, Hannes Frederic Sowa, Florian Westphal
In-Reply-To: <e33476125bdaf4619c221aa82c034831bbdce89b.1485250657.git.pabeni@redhat.com>
On Tue, 24 Jan 2017 10:40:13 +0100
Paolo Abeni <pabeni@redhat.com> wrote:
>
> +/* return true if we should preserve skb nfct before calling ndo_start_xmit() */
> +static inline bool netif_needs_ct(struct net_device *dev)
> +{
> + /* any kind of virtual device needs to preserve the ct entry */
> + return dev->features & NETIF_F_LLTX;
> +}
> +
I am not sure I fully understand the problem, and this looks like a special case
hack. You are overloading meaning of LLTX flag in a new way.
If you must do it then code this in place, and put comment there.
The addition of accessor function just obscures the purpose.
^ permalink raw reply
* Re: [PATCH net] net: dsa: Keep a reference count on ethernet_dev
From: David Miller @ 2017-01-24 17:39 UTC (permalink / raw)
To: f.fainelli; +Cc: netdev, andrew, vivien.didelot, linux-kernel
In-Reply-To: <20170121174055.27560-1-f.fainelli@gmail.com>
From: Florian Fainelli <f.fainelli@gmail.com>
Date: Sat, 21 Jan 2017 09:40:54 -0800
> of_find_net_device_by_node() just returns a reference to a net_device but does
> not increment its reference count, which means that the master network device
> can just vanish under our feet.
>
> Fixes: 83c0afaec7b7 ("net: dsa: Add new binding implementation")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
This is fine, except now this netdev is completely locked into place with
no way to dynamically unload it.
If someone tries to modunload the driver for this ethernet device,
their screen will fill up with warning messages indicating that the
reference taken here in the DSA code is not going away.
You need to implement a netdev notifier that tears down this DSA
instance during an unregister event and releases the ethernet_dev.
Similar to how we handle protocol addresses bound to a netdev, etc.
^ permalink raw reply
* Re: [PATCH net-next v5] bridge: multicast to unicast
From: David Miller @ 2017-01-24 17:41 UTC (permalink / raw)
To: linus.luessing-djzkFPsfvsizQB+pC5nmwQ
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
stephen-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ, nbd-Vt+b4OUoWG0,
nikolay-qUQiAmfTcIp+XZJcv9eMoEEOCMrvLtNR,
bridge-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170121200133.1864-1-linus.luessing-djzkFPsfvsizQB+pC5nmwQ@public.gmane.org>
From: Linus Lüssing <linus.luessing-djzkFPsfvsizQB+pC5nmwQ@public.gmane.org>
Date: Sat, 21 Jan 2017 21:01:32 +0100
> From: Felix Fietkau <nbd-Vt+b4OUoWG0@public.gmane.org>
>
> Implements an optional, per bridge port flag and feature to deliver
> multicast packets to any host on the according port via unicast
> individually. This is done by copying the packet per host and
> changing the multicast destination MAC to a unicast one accordingly.
>
> multicast-to-unicast works on top of the multicast snooping feature of
> the bridge. Which means unicast copies are only delivered to hosts which
> are interested in it and signalized this via IGMP/MLD reports
> previously.
>
> This feature is intended for interface types which have a more reliable
> and/or efficient way to deliver unicast packets than broadcast ones
> (e.g. wifi).
>
> However, it should only be enabled on interfaces where no IGMPv2/MLDv1
> report suppression takes place. This feature is disabled by default.
>
> The initial patch and idea is from Felix Fietkau.
>
> Signed-off-by: Felix Fietkau <nbd-Vt+b4OUoWG0@public.gmane.org>
> [linus.luessing-djzkFPsfvsizQB+pC5nmwQ@public.gmane.org: various bug + style fixes, commit message]
> Signed-off-by: Linus Lüssing <linus.luessing-djzkFPsfvsizQB+pC5nmwQ@public.gmane.org>
I'm not completely happy that things have progressed this way, however
I don't have a better solution to suggest.
^ permalink raw reply
* Re: [PATCH net-next 3/3] net/tcp-fastopen: Add new API support
From: Eric Dumazet @ 2017-01-24 17:42 UTC (permalink / raw)
To: Yuchung Cheng
Cc: Willy Tarreau, Wei Wang, netdev, David Miller, Eric Dumazet,
Wei Wang
In-Reply-To: <CAK6E8=eR5N-AV9sDJGwB-VKTojqSBRnPF6e7cK4jeavTzP97Hw@mail.gmail.com>
On Tue, 2017-01-24 at 09:26 -0800, Yuchung Cheng wrote:
> >
> > Do you think there's a compelling reason for adding a new option or
> > are you interested in a small patch to perform the change above ?
> I like the proposal especially other stack also uses TCP_FASTOPEN
> https://msdn.microsoft.com/en-us/library/windows/desktop/ms738596(v=vs.85).aspx
Problem is that might break existing applications that were using
TCP_FASTOPEN before a connect() (it was a NOP until now)
I prefer we use a separate new option to be 100% safe, not adding
regressions.
Only new applications, tested, will use this new feature at their risk.
^ permalink raw reply
* Re: [PATCH] net: phy: marvell: Add Wake from LAN support for 88E1510 PHY
From: David Miller @ 2017-01-24 17:44 UTC (permalink / raw)
To: houjingj; +Cc: netdev
In-Reply-To: <1485080456-6750-1-git-send-email-houjingj@marvell.com>
From: Jingju Hou <houjingj@marvell.com>
Date: Sun, 22 Jan 2017 18:20:56 +0800
> Signed-off-by: Jingju Hou <houjingj@marvell.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next 3/3] net/tcp-fastopen: Add new API support
From: Eric Dumazet @ 2017-01-24 17:44 UTC (permalink / raw)
To: Willy Tarreau
Cc: Wei Wang, netdev, David Miller, Eric Dumazet, Yuchung Cheng,
Wei Wang
In-Reply-To: <20170123211623.GE20894@1wt.eu>
On Mon, 2017-01-23 at 22:16 +0100, Willy Tarreau wrote:
> Hi Wei,
>
> first, thanks a lot for doing this, it's really awesome!
>
> I'm testing it on 4.9 on haproxy and I met a corner case : when I
> perform a connect() to a server and I have nothing to send, upon
> POLLOUT notification since I have nothing to send I simply probe the
> connection using connect() again to see if it returns EISCONN or
> anything else. But here now I'm seeing EINPROGRESS loops.
>
> To illustrate this, here's what I'm doing :
>
> :8000 :8001
> [ client ] ---> [ proxy ] ---> [ server ]
>
> The proxy is configured to enable TFO to the server and the server
> supports TFO as well. The proxy and the server are in fact two proxy
> instances in haproxy running in the same process for convenience.
>
> When I already have data to send here's what I'm seeing (so it works fine) :
>
> 06:29:16.861190 accept4(7, {sa_family=AF_INET, sin_port=htons(33986), sin_addr=inet_addr("192.168.0.176")}, [128->16], SOCK
> _NONBLOCK) = 9
> 06:29:16.861277 setsockopt(9, SOL_TCP, TCP_NODELAY, [1], 4) = 0
> 06:29:16.861342 accept4(7, 0x7ffd0d794430, [128], SOCK_NONBLOCK) = -1 EAGAIN (Resource temporarily unavailable)
> 06:29:16.861417 recvfrom(9, "BLAH\n", 7006, 0, NULL, NULL) = 5
> 06:29:16.861509 recvfrom(9, 0x2619329, 7001, 0, NULL, NULL) = -1 EAGAIN (Resource temporarily unavailable)
> 06:29:16.861657 socket(AF_INET, SOCK_STREAM, IPPROTO_TCP) = 10
> 06:29:16.861730 fcntl(10, F_SETFL, O_RDONLY|O_NONBLOCK) = 0
> 06:29:16.861779 setsockopt(10, SOL_TCP, TCP_NODELAY, [1], 4) = 0
> 06:29:16.861882 setsockopt(10, SOL_TCP, 0x1e /* TCP_??? */, [1], 4) = 0
> 06:29:16.861942 connect(10, {sa_family=AF_INET, sin_port=htons(8001), sin_addr=inet_addr("127.0.0.1")}, 16) = 0
> 06:29:16.862015 epoll_ctl(3, EPOLL_CTL_ADD, 9, {EPOLLIN|EPOLLRDHUP, {u32=9, u64=9}}) = 0
> 06:29:16.862072 epoll_wait(3, [], 200, 0) = 0
> 06:29:16.862126 sendto(10, "BLAH\n", 5, MSG_DONTWAIT|MSG_NOSIGNAL, NULL, 0) = 5
> 06:29:16.862281 epoll_wait(3, [{EPOLLIN, {u32=8, u64=8}}], 200, 0) = 1
> 06:29:16.862334 recvfrom(10, 0x26173a4, 8030, 0, NULL, NULL) = -1 EAGAIN (Resource temporarily unavailable)
> 06:29:16.862385 accept4(8, {sa_family=AF_INET, sin_port=htons(46760), sin_addr=inet_addr("127.0.0.1")}, [128->16], SOCK_NON
> BLOCK) = 11
> 06:29:16.862450 setsockopt(11, SOL_TCP, TCP_NODELAY, [1], 4) = 0
> 06:29:16.862504 accept4(8, 0x7ffd0d794430, [128], SOCK_NONBLOCK) = -1 EAGAIN (Resource temporarily unavailable)
> 06:29:16.862564 recvfrom(11, "BLAH\n", 7006, 0, NULL, NULL) = 5
>
>
> When I don't have data, here's what I'm seeing :
>
> 06:29:24.047801 accept4(7, {sa_family=AF_INET, sin_port=htons(33988), sin_addr=inet_addr("192.168.0.176")}, [128->16], SOCK
> _NONBLOCK) = 9
> 06:29:24.047899 setsockopt(9, SOL_TCP, TCP_NODELAY, [1], 4) = 0
> 06:29:24.047966 accept4(7, 0x7ffdedb2c7f0, [128], SOCK_NONBLOCK) = -1 EAGAIN (Resource temporarily unavailable)
> 06:29:24.048043 recvfrom(9, 0xd31324, 7006, 0, NULL, NULL) = -1 EAGAIN (Resource temporarily unavailable)
> 06:29:24.048281 socket(AF_INET, SOCK_STREAM, IPPROTO_TCP) = 10
> 06:29:24.048342 fcntl(10, F_SETFL, O_RDONLY|O_NONBLOCK) = 0
> 06:29:24.048392 setsockopt(10, SOL_TCP, TCP_NODELAY, [1], 4) = 0
> 06:29:24.048447 setsockopt(10, SOL_TCP, 0x1e /* TCP_??? */, [1], 4) = 0
> 06:29:24.048508 connect(10, {sa_family=AF_INET, sin_port=htons(8001), sin_addr=inet_addr("127.0.0.1")}, 16) = 0
I believe there is a bug in this application.
It does not check connect() return value.
When 0 is returned, it makes no sense to wait 200 ms :
> 06:29:24.048593 epoll_ctl(3, EPOLL_CTL_ADD, 9, {EPOLLIN|EPOLLRDHUP, {u32=9, u64=9}}) = 0
> 06:29:24.048651 epoll_wait(3, [], 200, 0) = 0
> 06:29:24.048699 getsockopt(10, SOL_SOCKET, SO_ERROR, [0], [4]) = 0
And it makes no sense to call connect() again :
> 06:29:24.048751 connect(10, {sa_family=AF_INET, sin_port=htons(8001), sin_addr=inet_addr("127.0.0.1")}, 16) = -1 EINPROGRES
> S (Operation now in progress)
man connect
<quote>
Generally, connection-based protocol sockets may successfully connect()
only once;
</quote>
I would prefer we do not add yet another bit in tcp kernel sockets, to
work around some oddity in your program Willy.
> 06:29:24.048808 epoll_ctl(3, EPOLL_CTL_ADD, 10, {EPOLLOUT, {u32=10, u64=10}}) = 0
> 06:29:24.048860 epoll_wait(3, [{EPOLLOUT, {u32=10, u64=10}}], 200, 1000) = 1
> 06:29:24.048912 getsockopt(10, SOL_SOCKET, SO_ERROR, [0], [4]) = 0
> 06:29:24.048963 connect(10, {sa_family=AF_INET, sin_port=htons(8001), sin_addr=inet_addr("127.0.0.1")}, 16) = -1 EINPROGRES
> S (Operation now in progress)
> 06:29:24.049018 epoll_wait(3, [{EPOLLOUT, {u32=10, u64=10}}], 200, 1000) = 1
> 06:29:24.049072 getsockopt(10, SOL_SOCKET, SO_ERROR, [0], [4]) = 0
> 06:29:24.049122 connect(10, {sa_family=AF_INET, sin_port=htons(8001), sin_addr=inet_addr("127.0.0.1")}, 16) = -1 EINPROGRES
> S (Operation now in progress)
>
>
> I theorically understand why but I think we have something wrong here
> and instead we should have -1 EISCONN (to pretend the connection is
> established) or return EALREADY (to mention that a previous request was
> already made and that we're waiting for the next step).
>
> While I can instrument my connect() *not* to use TFO when connecting
> without any pending data, I don't always know this (eg when I use
> openssl and cross fingers so that it decides to quickly send something
> on the next round).
>
> I think it's easy to fall into this tricky corner case and am wondering
> what can be done about it. Does the EINPROGRESS happen only because there
> is no cookie yet ? If so, shouldn't the connect's status change in this
> case ?
>
> Thanks,
> Willy
^ permalink raw reply
* Re: [PATCH net-next 5/5] net: dsa: mv88e6xxx: Implement the 6390 external MDIO bus
From: Vivien Didelot @ 2017-01-24 17:48 UTC (permalink / raw)
To: Andrew Lunn, David Miller; +Cc: netdev, Florian Fainelli, Andrew Lunn
In-Reply-To: <1485266031-4980-6-git-send-email-andrew@lunn.ch>
Hi Andrew,
Sorry but the previous patches with a list of MDIO busses and refactored
PHY ops still seems too much complex to just toggle a bit.
We know which switch port has an external PHY attached to it, right?
So why not just something like this:
struct mv88e6xxx_chip {
...
u16 external_phys;
...
}
int mv88e6xxx_g2_smi_phy_read(struct mv88e6xxx_chip *chip, int phy,
int reg, u16 *val)
{
u16 cmd = GLOBAL2_SMI_PHY_CMD_OP_22_READ_DATA | (addr << 5) | reg;
int err;
if (chip->external_phys & BIT(phy))
cmd |= GLOBAL2_SMI_PHY_CMD_EXTERNAL;
...
}
Thanks!
Vivien
^ permalink raw reply
* Re: [PATCH v2] bpf: Restrict cgroup bpf hooks to the init netns
From: Alexei Starovoitov @ 2017-01-24 17:48 UTC (permalink / raw)
To: Andy Lutomirski
Cc: David Ahern, Andy Lutomirski, Network Development,
David S. Miller, Daniel Borkmann, Alexei Starovoitov
In-Reply-To: <CALCETrVsqktjiPkfpcm_5uL0UL2Lzr106ZteLchJVnsKH_e-aw@mail.gmail.com>
On Mon, Jan 23, 2017 at 08:32:02PM -0800, Andy Lutomirski wrote:
> On Mon, Jan 23, 2017 at 8:05 PM, David Ahern <dsa@cumulusnetworks.com> wrote:
> > On 1/23/17 8:37 PM, Andy Lutomirski wrote:
> >> Yes, it is a bug because cgroup+bpf causes unwitting programs to be
> >> subject to BPF code installed by a different, potentially unrelated
> >> process. That's a new situation. The failure can happen when a
> >> privileged supervisor (whoever runs ip vrf) runs a clueless or
> >> unprivileged program (the thing calling unshare()).
> >
> > There are many, many ways to misconfigure networking and to run programs in a context or with an input argument that causes the program to not work at all, not work as expected or stop working. This situation is no different.
> >
> > For example, the only aspect of BPF_PROG_TYPE_CGROUP_SOCK filters that are namespace based is the ifindex. You brought up the example of changing namespaces where the ifindex is not defined. Alexei mentioned an example where interfaces can be moved to another namespace breaking any ifindex based programs. Another example is the interface can be deleted. Deleting an interface with sockets bound to it does not impact the program in any way - no notification, no wakeup, nothing. The sockets just don't work.
>
> And if you use 'ip vrf' to bind to a vrf with ifindex 4 and a program
> unshares netns and creates an interface with ifindex 4, then that
> program will end up with its sockets magically bound to ifindex 4 and
> will silently malfunction.
>
> I can think of multiple ways to address this problem. You could scope
> the hooks to a netns (which is sort of what my patch does). You could
> find a way to force programs in a given cgroup to only execute in a
> single netns, although that would probably cause other breakage. You
> could improve the BPF hook API to be netns-aware, which could plausbly
> address issues related to unshare() but might get very tricky when
> setns() is involved.
scoping cgroup to netns will create weird combination of cgroup and netns
which was never done before. cgroup and netns scopes have to be able
to overlap in arbitrary way. Application shouldn't not be able to escape
cgroup scope by changing netns. They are orthogonal scoping constructs.
What we can do instead is to do add bpf helper function to retrieve
the netns id, so that program created for specific netns and specific
ifindex will be more resilient to netns changes.
Note that this ifindex behavior has been present since classic bpf
and its shortcomings are understood. We need to improve it for all bpf
users. It really has nothing to do with cgroups.
cls_bpf filters can do packet redirect into ifindex and they can break
if user process changes.
The 'malfunction' scenario describe above is no different than
cls_bpf is using bpf_skb_under_cgroup() to scope particular
skb->sk->cgroup and doing redirect into ifindex.
It will 'malfunction' exactly the same way if application stays
in the same cgroup, but moves from one netns into another.
> My point is that, in 4.10-rc, it doesn't work right, and I doubt this
> problem is restricted to just 'ip vrf'. Without some kind of change
> to the way that netns and cgroup+bpf interact, anything that uses
> sk_bound_dev_if or reads the ifindex on an skb will be subject to a
> huge footgun that unprivileged programs can trigger and any future
> attempt to make the cgroup+bpf work for unprivileged users is going to
> be more complicated than it deserves to be.
For n-th time, the current BPF_PROG_TYPE_CGROUP* is root only and
speculation about unprivileged usage are not helping the discussion.
> things up so that unshare will malfunction. It should avoid
> malfunctioning when running Linux programs that are unaware of it.
I agree that for VRF use case it will help to make programs netns
aware by adding new bpf_get_current_netns_id() or something helper,
but it's up to the program to function properly or be broken.
Adding weird netns restrictions is not going to make bpf programs
any more sane. The program author can always shoot in the foot.
I will work on the patch to add that.
^ permalink raw reply
* Re: [for-next 07/10] IB/mlx5: Use blue flame register allocator in mlx5_ib
From: Doug Ledford @ 2017-01-24 17:49 UTC (permalink / raw)
To: Eli Cohen, Leon Romanovsky, David Miller
Cc: eli-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org,
Saeed Mahameed, netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Matan Barak
In-Reply-To: <AM4PR0501MB2787B803D95B7575065CC497C5750-dp/nxUn679jFcPxmzbbP+MDSnupUy6xnnBOFsp37pqbUKgpGm//BTAC/G2K4zDHf@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 453 bytes --]
On Tue, 2017-01-24 at 17:16 +0000, Eli Cohen wrote:
> It is in Dave's tree:
>
> commit 5fe9dec0d045437e48f112b8fa705197bd7bc3c0
> Author: Eli Cohen <eli-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Date: Tue Jan 3 23:55:25 2017 +0200
>
Thanks, got it.
--
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] bpf: don't kfree an uninitialized im_node
From: Alexei Starovoitov @ 2017-01-24 17:53 UTC (permalink / raw)
To: Colin King, Daniel Borkmann, David S. Miller
Cc: Alexei Starovoitov, netdev@vger.kernel.org, kernel-janitors,
linux-kernel@vger.kernel.org
In-Reply-To: <20170124141629.9200-1-colin.king@canonical.com>
On Tue, Jan 24, 2017 at 6:16 AM, Colin King <colin.king@canonical.com> wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> There are some error exit paths to the label 'out' that end up
> kfree'ing an uninitialized im_node. Fix this by inititializing
> im_node to NULL to avoid kfree'ing a garbage address.
this fix already landed. See:
commit d140199af510 ("bpf, lpm: fix kfree of im_node in trie_update_elem")
> Issue found by CoverityScan, CID#1398022 ("Uninitialized pointer read")
Nice. Good to know that static analysis can do such checks.
^ permalink raw reply
* Re: [PATCH net] net: dsa: Keep a reference count on ethernet_dev
From: Florian Fainelli @ 2017-01-24 17:56 UTC (permalink / raw)
To: David Miller; +Cc: netdev, andrew, vivien.didelot, linux-kernel
In-Reply-To: <20170124.123911.330293119907498159.davem@davemloft.net>
On 01/24/2017 09:39 AM, David Miller wrote:
> From: Florian Fainelli <f.fainelli@gmail.com>
> Date: Sat, 21 Jan 2017 09:40:54 -0800
>
>> of_find_net_device_by_node() just returns a reference to a net_device but does
>> not increment its reference count, which means that the master network device
>> can just vanish under our feet.
>>
>> Fixes: 83c0afaec7b7 ("net: dsa: Add new binding implementation")
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>
> This is fine, except now this netdev is completely locked into place with
> no way to dynamically unload it.
>
> If someone tries to modunload the driver for this ethernet device,
> their screen will fill up with warning messages indicating that the
> reference taken here in the DSA code is not going away.
>
> You need to implement a netdev notifier that tears down this DSA
> instance during an unregister event and releases the ethernet_dev.
> Similar to how we handle protocol addresses bound to a netdev, etc.
OK, that was actually my original approach, and then while detaching the
switch from the network device seemed easy enough, re-attaching it could
be a little challenging. Let's try again.
Thanks!
--
Florian
^ permalink raw reply
* Re: [PATCH v5 0/2] Add support for the ethernet switch on the ESPRESSObin
From: Vivien Didelot @ 2017-01-24 18:08 UTC (permalink / raw)
To: Gregory CLEMENT, David Miller
Cc: andrew, f.fainelli, netdev, linux-kernel, jason,
sebastian.hesselbarth, thomas.petazzoni, linux-arm-kernel, nadavh,
dingwei, kostap, shjzhou, jpannell
In-Reply-To: <87r33s2ynp.fsf@free-electrons.com>
Hi Gregory,
Gregory CLEMENT <gregory.clement@free-electrons.com> writes:
> Hi David,
>
> On ven., janv. 20 2017, David Miller <davem@davemloft.net> wrote:
>
>> From: Gregory CLEMENT <gregory.clement@free-electrons.com>
>> Date: Thu, 19 Jan 2017 22:49:32 +0100
>>
>>> I created a new family for this switch and filled the ops structure
>>> by selecting which seems the more appropriate functions. I rebased
>>> the series on net-next/master which allowed me to benefit to the
>>> eeprom functions introduced for the 6390.
>>
>> It looks like there will be at least one more respin of this series,
>> specifically to remove the new family as Vivien seems to object to
>> this.
>
> I am about to send a new version. However about removing the new family,
> I thought that with the confirmation from Andrew we could keep it.
>
> Vivien could you confirm this?
I confirm.
Thanks!
Vivien
^ permalink raw reply
* Re: [PATCH net-next v5] bridge: multicast to unicast
From: Stephen Hemminger @ 2017-01-24 18:10 UTC (permalink / raw)
To: Linus Lüssing
Cc: netdev, David S . Miller, Felix Fietkau, Nikolay Aleksandrov,
bridge, linux-kernel, linux-wireless
In-Reply-To: <20170121200133.1864-1-linus.luessing@c0d3.blue>
On Sat, 21 Jan 2017 21:01:32 +0100
Linus Lüssing <linus.luessing@c0d3.blue> wrote:
> @@ -726,12 +729,32 @@ struct net_bridge_port_group *br_multicast_new_port_group(
> hlist_add_head(&p->mglist, &port->mglist);
> setup_timer(&p->timer, br_multicast_port_group_expired,
> (unsigned long)p);
> +
> + if (src)
> + memcpy(p->eth_addr, src, ETH_ALEN);
> + else
> + memset(p->eth_addr, 0xff, ETH_ALEN);
> +
> return p;
> }
Maybe use ether_addr_copy here, but is not performance critical
^ permalink raw reply
* Re: [PATCH net-next v5] bridge: multicast to unicast
From: Stephen Hemminger @ 2017-01-24 18:12 UTC (permalink / raw)
To: Linus Lüssing
Cc: netdev, David S . Miller, Felix Fietkau, Nikolay Aleksandrov,
bridge, linux-kernel, linux-wireless
In-Reply-To: <20170121200133.1864-1-linus.luessing@c0d3.blue>
On Sat, 21 Jan 2017 21:01:32 +0100
Linus Lüssing <linus.luessing@c0d3.blue> wrote:
> +static bool br_port_group_equal(struct net_bridge_port_group *p,
> + struct net_bridge_port *port,
> + const unsigned char *src)
> +{
> + if (p->port != port)
> + return false;
> +
> + if (!(port->flags & BR_MULTICAST_TO_UNICAST))
> + return true;
> +
> + return ether_addr_equal(src, p->eth_addr);
> +}
> +
I prefer that argument to functions like this be const.
^ permalink raw reply
* [PATCH/RFC v3 net] ravb: unmap descriptors when freeing rings
From: Simon Horman @ 2017-01-24 18:21 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: Magnus Damm, netdev, linux-renesas-soc
From: Kazuya Mizuguchi <kazuya.mizuguchi.ks@renesas.com>
"swiotlb buffer is full" errors occur after repeated initialisation of a
device - f.e. suspend/resume or ip link set up/down. This is because memory
mapped using dma_map_single() in ravb_ring_format() and ravb_start_xmit()
is not released. Resolve this problem by unmapping descriptors when
freeing rings.
Note, ravb_tx_free() is moved but not otherwise modified by this patch.
Signed-off-by: Kazuya Mizuguchi <kazuya.mizuguchi.ks@renesas.com>
[simon: reworked]
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
--
v3 [Simon Horman]
* As suggested by Sergei Shtylyov
- consistently use le32_to_cpu(desc->dptr)
- Do not clear desc->ds_cc as it is not used
* Paramatise ravb_tx_free() to allow it to free non-transmitted buffers
v2 [Simon Horman]
* As suggested by Sergei Shtylyov
- Use dma_mapping_error() and rx_desc->ds_cc when unmapping RX descriptors;
this is consistent with the way that they are mapped
- Use ravb_tx_free() to clear TX descriptors
* Reduce scope of new local variable
v1 [Kazuya Mizuguchi]
---
drivers/net/ethernet/renesas/ravb_main.c | 113 ++++++++++++++++++-------------
1 file changed, 65 insertions(+), 48 deletions(-)
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 89ac1e3f6175..57fe1411bb9d 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -179,6 +179,51 @@ static struct mdiobb_ops bb_ops = {
.get_mdio_data = ravb_get_mdio_data,
};
+enum ravb_tx_free_mode {
+ ravb_tx_free_all,
+ ravb_tx_free_txed_only,
+};
+
+/* Free TX skb function for AVB-IP */
+static int ravb_tx_free(struct net_device *ndev, int q,
+ enum ravb_tx_free_mode free_mode)
+{
+ struct ravb_private *priv = netdev_priv(ndev);
+ struct net_device_stats *stats = &priv->stats[q];
+ struct ravb_tx_desc *desc;
+ int free_num = 0;
+ int entry;
+ u32 size;
+
+ for (; priv->cur_tx[q] - priv->dirty_tx[q] > 0; priv->dirty_tx[q]++) {
+ entry = priv->dirty_tx[q] % (priv->num_tx_ring[q] *
+ NUM_TX_DESC);
+ desc = &priv->tx_ring[q][entry];
+ if (free_mode == ravb_tx_free_txed_only &&
+ desc->die_dt != DT_FEMPTY)
+ break;
+ /* Descriptor type must be checked before all other reads */
+ dma_rmb();
+ size = le16_to_cpu(desc->ds_tagl) & TX_DS;
+ /* Free the original skb. */
+ if (priv->tx_skb[q][entry / NUM_TX_DESC]) {
+ dma_unmap_single(ndev->dev.parent, le32_to_cpu(desc->dptr),
+ size, DMA_TO_DEVICE);
+ /* Last packet descriptor? */
+ if (entry % NUM_TX_DESC == NUM_TX_DESC - 1) {
+ entry /= NUM_TX_DESC;
+ dev_kfree_skb_any(priv->tx_skb[q][entry]);
+ priv->tx_skb[q][entry] = NULL;
+ stats->tx_packets++;
+ }
+ free_num++;
+ }
+ stats->tx_bytes += size;
+ desc->die_dt = DT_EEMPTY;
+ }
+ return free_num;
+}
+
/* Free skb's and DMA buffers for Ethernet AVB */
static void ravb_ring_free(struct net_device *ndev, int q)
{
@@ -194,19 +239,21 @@ static void ravb_ring_free(struct net_device *ndev, int q)
kfree(priv->rx_skb[q]);
priv->rx_skb[q] = NULL;
- /* Free TX skb ringbuffer */
- if (priv->tx_skb[q]) {
- for (i = 0; i < priv->num_tx_ring[q]; i++)
- dev_kfree_skb(priv->tx_skb[q][i]);
- }
- kfree(priv->tx_skb[q]);
- priv->tx_skb[q] = NULL;
-
/* Free aligned TX buffers */
kfree(priv->tx_align[q]);
priv->tx_align[q] = NULL;
if (priv->rx_ring[q]) {
+ for (i = 0; i < priv->num_rx_ring[q]; i++) {
+ struct ravb_ex_rx_desc *desc = &priv->rx_ring[q][i];
+
+ if (!dma_mapping_error(ndev->dev.parent,
+ le32_to_cpu(desc->dptr)))
+ dma_unmap_single(ndev->dev.parent,
+ le32_to_cpu(desc->dptr),
+ PKT_BUF_SZ,
+ DMA_FROM_DEVICE);
+ }
ring_size = sizeof(struct ravb_ex_rx_desc) *
(priv->num_rx_ring[q] + 1);
dma_free_coherent(ndev->dev.parent, ring_size, priv->rx_ring[q],
@@ -215,12 +262,19 @@ static void ravb_ring_free(struct net_device *ndev, int q)
}
if (priv->tx_ring[q]) {
+ ravb_tx_free(ndev, q, ravb_tx_free_all);
+
ring_size = sizeof(struct ravb_tx_desc) *
(priv->num_tx_ring[q] * NUM_TX_DESC + 1);
dma_free_coherent(ndev->dev.parent, ring_size, priv->tx_ring[q],
priv->tx_desc_dma[q]);
priv->tx_ring[q] = NULL;
}
+
+ /* Free TX skb ringbuffer.
+ * SKBs are freed by ravb_tx_free() call above. */
+ kfree(priv->tx_skb[q]);
+ priv->tx_skb[q] = NULL;
}
/* Format skb and descriptor buffer for Ethernet AVB */
@@ -431,44 +485,6 @@ static int ravb_dmac_init(struct net_device *ndev)
return 0;
}
-/* Free TX skb function for AVB-IP */
-static int ravb_tx_free(struct net_device *ndev, int q)
-{
- struct ravb_private *priv = netdev_priv(ndev);
- struct net_device_stats *stats = &priv->stats[q];
- struct ravb_tx_desc *desc;
- int free_num = 0;
- int entry;
- u32 size;
-
- for (; priv->cur_tx[q] - priv->dirty_tx[q] > 0; priv->dirty_tx[q]++) {
- entry = priv->dirty_tx[q] % (priv->num_tx_ring[q] *
- NUM_TX_DESC);
- desc = &priv->tx_ring[q][entry];
- if (desc->die_dt != DT_FEMPTY)
- break;
- /* Descriptor type must be checked before all other reads */
- dma_rmb();
- size = le16_to_cpu(desc->ds_tagl) & TX_DS;
- /* Free the original skb. */
- if (priv->tx_skb[q][entry / NUM_TX_DESC]) {
- dma_unmap_single(ndev->dev.parent, le32_to_cpu(desc->dptr),
- size, DMA_TO_DEVICE);
- /* Last packet descriptor? */
- if (entry % NUM_TX_DESC == NUM_TX_DESC - 1) {
- entry /= NUM_TX_DESC;
- dev_kfree_skb_any(priv->tx_skb[q][entry]);
- priv->tx_skb[q][entry] = NULL;
- stats->tx_packets++;
- }
- free_num++;
- }
- stats->tx_bytes += size;
- desc->die_dt = DT_EEMPTY;
- }
- return free_num;
-}
-
static void ravb_get_tx_tstamp(struct net_device *ndev)
{
struct ravb_private *priv = netdev_priv(ndev);
@@ -902,7 +918,7 @@ static int ravb_poll(struct napi_struct *napi, int budget)
spin_lock_irqsave(&priv->lock, flags);
/* Clear TX interrupt */
ravb_write(ndev, ~mask, TIS);
- ravb_tx_free(ndev, q);
+ ravb_tx_free(ndev, q, ravb_tx_free_txed_only);
netif_wake_subqueue(ndev, q);
mmiowb();
spin_unlock_irqrestore(&priv->lock, flags);
@@ -1567,7 +1583,8 @@ static netdev_tx_t ravb_start_xmit(struct sk_buff *skb, struct net_device *ndev)
priv->cur_tx[q] += NUM_TX_DESC;
if (priv->cur_tx[q] - priv->dirty_tx[q] >
- (priv->num_tx_ring[q] - 1) * NUM_TX_DESC && !ravb_tx_free(ndev, q))
+ (priv->num_tx_ring[q] - 1) * NUM_TX_DESC &&
+ !ravb_tx_free(ndev, q, ravb_tx_free_txed_only))
netif_stop_subqueue(ndev, q);
exit:
--
2.7.0.rc3.207.g0ac5344
^ permalink raw reply related
* Re: [PATCH] Documentation: net: phy: improve explanation when to specify the PHY ID
From: David Miller @ 2017-01-24 18:31 UTC (permalink / raw)
To: martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg
Cc: andrew-g2DYL2Zd6BY, devicetree-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA, f.fainelli-Re5JQEeQqe8AvxtiuMwx3w,
mark.rutland-5wv7dgnIgG8, robh+dt-DgEjT+Ai2ygdnm+yROfE0A
In-Reply-To: <20170122164132.13967-1-martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
From: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
Date: Sun, 22 Jan 2017 17:41:32 +0100
> The old description basically read like "ethernet-phy-idAAAA.BBBB" can
> be specified when you know the actual PHY ID. However, specifying this
> has a side-effect: it forces Linux to bind to a certain PHY driver (the
> one that matches the ID given in the compatible string), ignoring the ID
> which is reported by the actual PHY.
> Whenever a device is shipped with (multiple) different PHYs during it's
> production lifetime then explicitly specifying
> "ethernet-phy-idAAAA.BBBB" could break certain revisions of that device.
>
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> ---
> Thanks to Andrew Lunn for pointing the documentation issue out to me in:
> http://lists.infradead.org/pipermail/linux-amlogic/2017-January/002141.html
Applied.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox