* Re: [PATCH v2] kcm: remove any offset before parsing messages
From: Dominique Martinet @ 2018-09-18 2:58 UTC (permalink / raw)
To: David Miller; +Cc: doronrk, tom, davejwatson, netdev, linux-kernel
In-Reply-To: <20180917.195150.315319338322641005.davem@davemloft.net>
David Miller wrote on Mon, Sep 17, 2018:
> > No, they can see it, so it's possible to make a KCM program that works
> > right now if you are careful (I'm not sure why the offset within bpf is
> > different from the offset in the kernel though, it looks like the bpf
> > program skips the qos part of the control buffer)
>
> What helper is used in the BPF program to get this offset value?
>
> (also good info to add to the commit message)
Dave defined one himself ; for a simple protocol where the offset is in
the first four bytes of the message.
The whole bpf program could look like this:
------
struct kcm_rx_msg { int full_len; int offset; };
static inline struct kcm_rx_msg *kcm_rx_msg(struct __sk_buff *skb) {
return (struct kcm_rx_msg *)skb->cb;
}
int decode_framing(struct __sk_buff *skb) {
return load_word(skb, kcm_rx_msg(skb)->offset);
}
------
If we go towards documenting it, adding a helper would be useful yes;
buf if we pull that becomes unnecessary.
(I'll add the example program in the commit message anyway at your
suggestion)
--
Dominique Martinet
^ permalink raw reply
* [PATCH net-next] qed: remove duplicated include from qed_cxt.c
From: YueHaibing @ 2018-09-18 2:54 UTC (permalink / raw)
To: davem, Ariel.Elior, everest-linux-l2; +Cc: linux-kernel, netdev, YueHaibing
Remove duplicated include.
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
drivers/net/ethernet/qlogic/qed/qed_cxt.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/ethernet/qlogic/qed/qed_cxt.c b/drivers/net/ethernet/qlogic/qed/qed_cxt.c
index f1977aa..dc1c1b6 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_cxt.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_cxt.c
@@ -40,7 +40,6 @@
#include <linux/pci.h>
#include <linux/slab.h>
#include <linux/string.h>
-#include <linux/bitops.h>
#include "qed.h"
#include "qed_cxt.h"
#include "qed_dev_api.h"
--
2.7.0
^ permalink raw reply related
* Re: [PATCH 3/4] qed: remove duplicated include from qed_cxt.c
From: David Miller @ 2018-09-18 2:53 UTC (permalink / raw)
To: yuehaibing; +Cc: Ariel.Elior, everest-linux-l2, linux-kernel, netdev
In-Reply-To: <20180918024627.10320-1-yuehaibing@huawei.com>
From: YueHaibing <yuehaibing@huawei.com>
Date: Tue, 18 Sep 2018 10:46:27 +0800
> Remove duplicated include.
>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Applied.
^ permalink raw reply
* Re: [PATCH v2] kcm: remove any offset before parsing messages
From: David Miller @ 2018-09-18 2:51 UTC (permalink / raw)
To: asmadeus; +Cc: doronrk, tom, davejwatson, netdev, linux-kernel
In-Reply-To: <20180918024536.GA2061@nautica>
From: Dominique Martinet <asmadeus@codewreck.org>
Date: Tue, 18 Sep 2018 04:45:36 +0200
> David Miller wrote on Mon, Sep 17, 2018:
>> Remind me, is there actually any way for the bpf programs run in this
>> situation to even _see_ strp_msg(skb)->offset at all?
>
> No, they can see it, so it's possible to make a KCM program that works
> right now if you are careful (I'm not sure why the offset within bpf is
> different from the offset in the kernel though, it looks like the bpf
> program skips the qos part of the control buffer)
What helper is used in the BPF program to get this offset value?
(also good info to add to the commit message)
^ permalink raw reply
* Re: [bpf PATCH 3/3] bpf: test_maps, only support ESTABLISHED socks
From: Y Song @ 2018-09-17 21:21 UTC (permalink / raw)
To: John Fastabend; +Cc: edumazet, Alexei Starovoitov, Daniel Borkmann, netdev
In-Reply-To: <20180917173200.21218.51219.stgit@john-Precision-Tower-5810>
On Mon, Sep 17, 2018 at 10:33 AM John Fastabend
<john.fastabend@gmail.com> wrote:
>
> Ensure that sockets added to a sock{map|hash} that is not in the
> ESTABLISHED state is rejected.
>
> Fixes: 1aa12bdf1bfb ("bpf: sockmap, add sock close() hook to remove socks")
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
> ---
> tools/testing/selftests/bpf/test_maps.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/test_maps.c b/tools/testing/selftests/bpf/test_maps.c
> index 6f54f84..0f2090f 100644
> --- a/tools/testing/selftests/bpf/test_maps.c
> +++ b/tools/testing/selftests/bpf/test_maps.c
> @@ -580,7 +580,11 @@ static void test_sockmap(int tasks, void *data)
> /* Test update without programs */
> for (i = 0; i < 6; i++) {
> err = bpf_map_update_elem(fd, &i, &sfd[i], BPF_ANY);
> - if (err) {
> + if (i < 2 && !err) {
> + printf("Allowed update sockmap '%i:%i' not in ESTABLISHED\n",
> + i, sfd[i]);
> + goto out_sockmap;
> + } else if (i > 1 && err) {
Just a nit. Maybe "i >= 2" since it will be more clear since it is
opposite of "i < 2"?
> printf("Failed noprog update sockmap '%i:%i'\n",
> i, sfd[i]);
> goto out_sockmap;
> @@ -741,7 +745,7 @@ static void test_sockmap(int tasks, void *data)
> }
>
> /* Test map update elem afterwards fd lives in fd and map_fd */
> - for (i = 0; i < 6; i++) {
> + for (i = 2; i < 6; i++) {
> err = bpf_map_update_elem(map_fd_rx, &i, &sfd[i], BPF_ANY);
> if (err) {
> printf("Failed map_fd_rx update sockmap %i '%i:%i'\n",
> @@ -845,7 +849,7 @@ static void test_sockmap(int tasks, void *data)
> }
>
> /* Delete the elems without programs */
> - for (i = 0; i < 6; i++) {
> + for (i = 2; i < 6; i++) {
> err = bpf_map_delete_elem(fd, &i);
> if (err) {
> printf("Failed delete sockmap %i '%i:%i'\n",
>
^ permalink raw reply
* Re: [PATCH net-next] liquidio: remove duplicated include from lio_vf_rep.c
From: David Miller @ 2018-09-18 2:50 UTC (permalink / raw)
To: yuehaibing
Cc: derek.chickles, satananda.burla, felix.manlunas, raghu.vatsavayi,
linux-kernel, netdev
In-Reply-To: <20180918024343.19832-1-yuehaibing@huawei.com>
From: YueHaibing <yuehaibing@huawei.com>
Date: Tue, 18 Sep 2018 10:43:43 +0800
> Remove duplicated include.
>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Applied.
^ permalink raw reply
* Re: [PATCH 3/4] qed: remove duplicated include from qed_cxt.c
From: YueHaibing @ 2018-09-18 2:50 UTC (permalink / raw)
To: davem, Ariel.Elior, everest-linux-l2; +Cc: linux-kernel, netdev
In-Reply-To: <20180918024627.10320-1-yuehaibing@huawei.com>
sorry for the title, it should be net-next.
On 2018/9/18 10:46, YueHaibing wrote:
> Remove duplicated include.
>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> ---
> drivers/net/ethernet/qlogic/qed/qed_cxt.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/qlogic/qed/qed_cxt.c b/drivers/net/ethernet/qlogic/qed/qed_cxt.c
> index f1977aa..dc1c1b6 100644
> --- a/drivers/net/ethernet/qlogic/qed/qed_cxt.c
> +++ b/drivers/net/ethernet/qlogic/qed/qed_cxt.c
> @@ -40,7 +40,6 @@
> #include <linux/pci.h>
> #include <linux/slab.h>
> #include <linux/string.h>
> -#include <linux/bitops.h>
> #include "qed.h"
> #include "qed_cxt.h"
> #include "qed_dev_api.h"
>
^ permalink raw reply
* [PATCH net-next] net: mdio: remove duplicated include from mdio_bus.c
From: YueHaibing @ 2018-09-18 2:48 UTC (permalink / raw)
To: davem, andrew, f.fainelli; +Cc: linux-kernel, netdev, YueHaibing
Remove duplicated include linux/gpio/consumer.h
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
drivers/net/phy/mdio_bus.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index 98f4b1f..2e59a84 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -38,7 +38,6 @@
#include <linux/phy.h>
#include <linux/io.h>
#include <linux/uaccess.h>
-#include <linux/gpio/consumer.h>
#include <asm/irq.h>
--
2.7.0
^ permalink raw reply related
* [PATCH 3/4] qed: remove duplicated include from qed_cxt.c
From: YueHaibing @ 2018-09-18 2:46 UTC (permalink / raw)
To: davem, Ariel.Elior, everest-linux-l2; +Cc: linux-kernel, netdev, YueHaibing
Remove duplicated include.
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
drivers/net/ethernet/qlogic/qed/qed_cxt.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/ethernet/qlogic/qed/qed_cxt.c b/drivers/net/ethernet/qlogic/qed/qed_cxt.c
index f1977aa..dc1c1b6 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_cxt.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_cxt.c
@@ -40,7 +40,6 @@
#include <linux/pci.h>
#include <linux/slab.h>
#include <linux/string.h>
-#include <linux/bitops.h>
#include "qed.h"
#include "qed_cxt.h"
#include "qed_dev_api.h"
--
2.7.0
^ permalink raw reply related
* Re: [PATCH v2] kcm: remove any offset before parsing messages
From: Dominique Martinet @ 2018-09-18 2:45 UTC (permalink / raw)
To: David Miller; +Cc: doronrk, tom, davejwatson, netdev, linux-kernel
In-Reply-To: <20180917.194059.1970452340378032090.davem@davemloft.net>
David Miller wrote on Mon, Sep 17, 2018:
> Remind me, is there actually any way for the bpf programs run in this
> situation to even _see_ strp_msg(skb)->offset at all?
No, they can see it, so it's possible to make a KCM program that works
right now if you are careful (I'm not sure why the offset within bpf is
different from the offset in the kernel though, it looks like the bpf
program skips the qos part of the control buffer)
> There isn't right? And the alternate proposal was to add such a
> facility, right?
The problem is that this isn't documented at all, and I could not find
any example doing that until Dave gave me one (I couldn't get it to work
because of the different offset).
The alternate proposal was to just document it, yes.
> Just trying to remember all of the context, maybe it's good
> information to add to the commit message?
Good idea, I'll add some more explanation there.
--
Dominique Martinet
^ permalink raw reply
* Re: [PATCH net-next] cxgb4: remove duplicated include from cxgb4_main.c
From: David Miller @ 2018-09-18 2:44 UTC (permalink / raw)
To: yuehaibing; +Cc: ganeshgr, linux-kernel, netdev
In-Reply-To: <20180918024128.2708-1-yuehaibing@huawei.com>
From: YueHaibing <yuehaibing@huawei.com>
Date: Tue, 18 Sep 2018 10:41:28 +0800
> Remove duplicated include.
>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Applied, thanks.
^ permalink raw reply
* [PATCH net-next] liquidio: remove duplicated include from lio_vf_rep.c
From: YueHaibing @ 2018-09-18 2:43 UTC (permalink / raw)
To: davem, derek.chickles, satananda.burla, felix.manlunas,
raghu.vatsavayi
Cc: linux-kernel, netdev, YueHaibing
Remove duplicated include.
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
drivers/net/ethernet/cavium/liquidio/lio_vf_rep.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/ethernet/cavium/liquidio/lio_vf_rep.c b/drivers/net/ethernet/cavium/liquidio/lio_vf_rep.c
index a9306164..96cf4a4 100644
--- a/drivers/net/ethernet/cavium/liquidio/lio_vf_rep.c
+++ b/drivers/net/ethernet/cavium/liquidio/lio_vf_rep.c
@@ -27,7 +27,6 @@
#include "octeon_network.h"
#include <net/switchdev.h>
#include "lio_vf_rep.h"
-#include "octeon_network.h"
static int lio_vf_rep_open(struct net_device *ndev);
static int lio_vf_rep_stop(struct net_device *ndev);
--
2.7.0
^ permalink raw reply related
* Re: [PATCH net-next] gianfar: remove duplicated include from gianfar.c
From: David Miller @ 2018-09-18 2:41 UTC (permalink / raw)
To: yuehaibing; +Cc: claudiu.manoil, linux-kernel, netdev
In-Reply-To: <20180918021718.24400-1-yuehaibing@huawei.com>
From: YueHaibing <yuehaibing@huawei.com>
Date: Tue, 18 Sep 2018 10:17:18 +0800
> Remove duplicated include.
>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Applied, thanks.
^ permalink raw reply
* [PATCH net-next] cxgb4: remove duplicated include from cxgb4_main.c
From: YueHaibing @ 2018-09-18 2:41 UTC (permalink / raw)
To: davem, ganeshgr; +Cc: linux-kernel, netdev, YueHaibing
Remove duplicated include.
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
index 2e1e286..1a93efa 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
@@ -62,7 +62,6 @@
#include <net/netevent.h>
#include <net/addrconf.h>
#include <net/bonding.h>
-#include <net/addrconf.h>
#include <linux/uaccess.h>
#include <linux/crash_dump.h>
#include <net/udp_tunnel.h>
--
2.7.0
^ permalink raw reply related
* Re: [PATCH v2] kcm: remove any offset before parsing messages
From: David Miller @ 2018-09-18 2:40 UTC (permalink / raw)
To: asmadeus; +Cc: doronrk, tom, davejwatson, netdev, linux-kernel
In-Reply-To: <20180918015723.GA26300@nautica>
From: Dominique Martinet <asmadeus@codewreck.org>
Date: Tue, 18 Sep 2018 03:57:23 +0200
> Given you did reply now I'll try to spend some time to figure that out
> in the next couple of weeks but it might not make it for this cycle
> depending on the number of rc we'll get and time you want this to soak
> it -next.
Great.
Remind me, is there actually any way for the bpf programs run in this
situation to even _see_ strp_msg(skb)->offset at all?
There isn't right? And the alternate proposal was to add such a
facility, right?
Just trying to remember all of the context, maybe it's good
information to add to the commit message?
^ permalink raw reply
* Re: [RFC/fix] Re: libbpf build broken on musl libc (Alpine Linux)
From: Jakub Kicinski @ 2018-09-18 2:39 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Arnaldo Carvalho de Melo, Daniel Borkmann, Thomas Richter,
Hendrik Brueckner, Jiri Olsa, Namhyung Kim, linux-kernel, netdev
In-Reply-To: <20180918005256.7uutwda4s3ofoxpd@ast-mbp>
On Mon, 17 Sep 2018 17:52:59 -0700, Alexei Starovoitov wrote:
> LGPL-2.1 in the above?
Could we possibly make it dual licensed LGPL + BSD?
> The rest looks good to me.
> Should we take it via bpf-next tree?
> If you feel there is an urgency to fix musl build, we can take it via
> bpf tree too.
>
> Jakub, thoughts? you've been messing with strerror last..
Sorry for the mess, I didn't know libc compat is such a pain.
I kind of knowingly ignored the existing str_error_r(), as it didn't
seem excessively clean. Yet, two months later I have no better
ideas.. We could as well switch back to the XSI version, which we used
before I needed access to reallocarray() (I mean change the str_error()
to be a plain XSI wrapper).
Perhaps placing the new helper in libbpf_errno.c file would make
sense? The only reason for this separate file to exist is in fact to
make use of XSI-compliant strerror_r().
Also, I need to go relicense tools/include/tools/libc_compat.h ASAP
too :S
^ permalink raw reply
* Re: [PATCH v2 net] net/ipv4: defensive cipso option parsing
From: David Miller @ 2018-09-18 2:38 UTC (permalink / raw)
To: snu; +Cc: netdev, aams, dwmw, yujuan.qi, paul, sveith, stable
In-Reply-To: <20180917174653.17046-1-snu@amazon.com>
From: Stefan Nuernberger <snu@amazon.com>
Date: Mon, 17 Sep 2018 19:46:53 +0200
> commit 40413955ee26 ("Cipso: cipso_v4_optptr enter infinite loop") fixed
> a possible infinite loop in the IP option parsing of CIPSO. The fix
> assumes that ip_options_compile filtered out all zero length options and
> that no other one-byte options beside IPOPT_END and IPOPT_NOOP exist.
> While this assumption currently holds true, add explicit checks for zero
> length and invalid length options to be safe for the future. Even though
> ip_options_compile should have validated the options, the introduction of
> new one-byte options can still confuse this code without the additional
> checks.
>
> Signed-off-by: Stefan Nuernberger <snu@amazon.com>
Applied to net-next.
This is not 'net' nor -stable material. I'm hesitent about this
change as-is, and ip_options_compile() is not changing semantics in
-stable in the way that you say can cause problems.
^ permalink raw reply
* Re: [bpf PATCH 2/3] bpf: sockmap, fix transition through disconnect without close
From: Y Song @ 2018-09-17 21:09 UTC (permalink / raw)
To: John Fastabend; +Cc: edumazet, Alexei Starovoitov, Daniel Borkmann, netdev
In-Reply-To: <20180917173155.21218.11026.stgit@john-Precision-Tower-5810>
On Mon, Sep 17, 2018 at 10:32 AM John Fastabend
<john.fastabend@gmail.com> wrote:
>
> It is possible (via shutdown()) for TCP socks to go trough TCP_CLOSE
> state via tcp_disconnect() without actually calling tcp_close which
> would then call our bpf_tcp_close() callback. Because of this a user
> could disconnect a socket then put it in a LISTEN state which would
> break our assumptions about sockets always being ESTABLISHED state.
>
> To resolve this rely on the unhash hook, which is called in the
> disconnect case, to remove the sock from the sockmap.
>
> Reported-by: Eric Dumazet <edumazet@google.com>
> Fixes: 1aa12bdf1bfb ("bpf: sockmap, add sock close() hook to remove socks")
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
> ---
> kernel/bpf/sockmap.c | 71 +++++++++++++++++++++++++++++++++++++-------------
> 1 file changed, 52 insertions(+), 19 deletions(-)
>
> diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c
> index 998b7bd..f6ab7f3 100644
> --- a/kernel/bpf/sockmap.c
> +++ b/kernel/bpf/sockmap.c
> @@ -132,6 +132,7 @@ struct smap_psock {
> struct work_struct gc_work;
>
> struct proto *sk_proto;
> + void (*save_unhash)(struct sock *sk);
> void (*save_close)(struct sock *sk, long timeout);
> void (*save_data_ready)(struct sock *sk);
> void (*save_write_space)(struct sock *sk);
> @@ -143,6 +144,7 @@ static int bpf_tcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
> static int bpf_tcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t size);
> static int bpf_tcp_sendpage(struct sock *sk, struct page *page,
> int offset, size_t size, int flags);
> +static void bpf_tcp_unhash(struct sock *sk);
> static void bpf_tcp_close(struct sock *sk, long timeout);
>
> static inline struct smap_psock *smap_psock_sk(const struct sock *sk)
> @@ -184,6 +186,7 @@ static void build_protos(struct proto prot[SOCKMAP_NUM_CONFIGS],
> struct proto *base)
> {
> prot[SOCKMAP_BASE] = *base;
> + prot[SOCKMAP_BASE].unhash = bpf_tcp_unhash;
> prot[SOCKMAP_BASE].close = bpf_tcp_close;
> prot[SOCKMAP_BASE].recvmsg = bpf_tcp_recvmsg;
> prot[SOCKMAP_BASE].stream_memory_read = bpf_tcp_stream_read;
> @@ -217,6 +220,7 @@ static int bpf_tcp_init(struct sock *sk)
> return -EBUSY;
> }
>
> + psock->save_unhash = sk->sk_prot->unhash;
> psock->save_close = sk->sk_prot->close;
> psock->sk_proto = sk->sk_prot;
>
> @@ -305,30 +309,12 @@ static struct smap_psock_map_entry *psock_map_pop(struct sock *sk,
> return e;
> }
>
> -static void bpf_tcp_close(struct sock *sk, long timeout)
> +static void bpf_tcp_remove(struct sock *sk, struct smap_psock *psock)
> {
> - void (*close_fun)(struct sock *sk, long timeout);
> struct smap_psock_map_entry *e;
> struct sk_msg_buff *md, *mtmp;
> - struct smap_psock *psock;
> struct sock *osk;
>
> - lock_sock(sk);
> - rcu_read_lock();
> - psock = smap_psock_sk(sk);
> - if (unlikely(!psock)) {
> - rcu_read_unlock();
> - release_sock(sk);
> - return sk->sk_prot->close(sk, timeout);
> - }
> -
> - /* The psock may be destroyed anytime after exiting the RCU critial
> - * section so by the time we use close_fun the psock may no longer
> - * be valid. However, bpf_tcp_close is called with the sock lock
> - * held so the close hook and sk are still valid.
> - */
> - close_fun = psock->save_close;
> -
> if (psock->cork) {
> free_start_sg(psock->sock, psock->cork, true);
> kfree(psock->cork);
> @@ -379,6 +365,53 @@ static void bpf_tcp_close(struct sock *sk, long timeout)
> kfree(e);
> e = psock_map_pop(sk, psock);
> }
> +}
> +
> +static void bpf_tcp_unhash(struct sock *sk)
> +{
> + void (*unhash_fun)(struct sock *sk);
> + struct smap_psock *psock;
> +
> + rcu_read_lock();
> + psock = smap_psock_sk(sk);
> + if (unlikely(!psock)) {
> + rcu_read_unlock();
> + release_sock(sk);
Can socket be released here?
> + return sk->sk_prot->unhash(sk);
> + }
> +
> + /* The psock may be destroyed anytime after exiting the RCU critial
> + * section so by the time we use close_fun the psock may no longer
> + * be valid. However, bpf_tcp_close is called with the sock lock
> + * held so the close hook and sk are still valid.
> + */
the comments above are not correct. A copy-paste mistake?
> + unhash_fun = psock->save_unhash;
> + bpf_tcp_remove(sk, psock);
> + rcu_read_unlock();
> + unhash_fun(sk);
> +}
> +
> +static void bpf_tcp_close(struct sock *sk, long timeout)
> +{
> + void (*close_fun)(struct sock *sk, long timeout);
> + struct smap_psock *psock;
> +
> + lock_sock(sk);
> + rcu_read_lock();
> + psock = smap_psock_sk(sk);
> + if (unlikely(!psock)) {
> + rcu_read_unlock();
> + release_sock(sk);
> + return sk->sk_prot->close(sk, timeout);
> + }
> +
> + /* The psock may be destroyed anytime after exiting the RCU critial
> + * section so by the time we use close_fun the psock may no longer
> + * be valid. However, bpf_tcp_close is called with the sock lock
> + * held so the close hook and sk are still valid.
> + */
> + close_fun = psock->save_close;
> + bpf_tcp_remove(sk, psock);
> rcu_read_unlock();
> release_sock(sk);
> close_fun(sk, timeout);
>
^ permalink raw reply
* Re: [PATCH] net: apm: xgene: force XGene enet driver to re-balance IRQ usage
From: David Miller @ 2018-09-18 2:35 UTC (permalink / raw)
To: ahs3; +Cc: netdev, linux-kernel, isubramanian, kchudgar, qnguyen
In-Reply-To: <20180917233533.28626-1-ahs3@redhat.com>
From: Al Stone <ahs3@redhat.com>
Date: Mon, 17 Sep 2018 17:35:33 -0600
> @@ -866,8 +866,11 @@ static int xgene_enet_napi(struct napi_struct *napi, const int budget)
> processed = xgene_enet_process_ring(ring, budget);
>
> if (processed != budget) {
> + struct irq_desc *desc = irq_to_desc(ring->irq);
> +
> napi_complete_done(napi, processed);
> - enable_irq(ring->irq);
> + if (desc && desc->depth > 0)
> + enable_irq(ring->irq);
I really can't accept a patch that grovels into IRQ layer internals
to work around a driver's IRQ enable/disable usage problem.
Sorry.
^ permalink raw reply
* Re: [bpf PATCH 1/3] bpf: sockmap only allow ESTABLISHED sock state
From: Y Song @ 2018-09-17 20:49 UTC (permalink / raw)
To: John Fastabend; +Cc: edumazet, Alexei Starovoitov, Daniel Borkmann, netdev
In-Reply-To: <20180917173149.21218.31436.stgit@john-Precision-Tower-5810>
On Mon, Sep 17, 2018 at 10:32 AM John Fastabend
<john.fastabend@gmail.com> wrote:
>
> After this patch we only allow socks that are in ESTABLISHED state or
> are being added via a sock_ops event that is transitioning into an
> ESTABLISHED state. By allowing sock_ops events we allow users to
> manage sockmaps directly from sock ops programs. The two supported
> sock_ops ops are BPF_SOCK_OPS_PASSIVE_ESTABLISHED_CB and
> BPF_SOCK_OPS_ACTIVE_ESTABLISHED_CB.
>
> Similar to TLS ULP this ensures sk_user_data is correct.
>
> Reported-by: Eric Dumazet <edumazet@google.com>
> Fixes: 1aa12bdf1bfb ("bpf: sockmap, add sock close() hook to remove socks")
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
Acked-by: Yonghong Song <yhs@fb.com>
^ permalink raw reply
* [PATCH net-next] gianfar: remove duplicated include from gianfar.c
From: YueHaibing @ 2018-09-18 2:17 UTC (permalink / raw)
To: claudiu.manoil, davem; +Cc: linux-kernel, netdev, YueHaibing
Remove duplicated include.
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
drivers/net/ethernet/freescale/gianfar.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index 40a1a87..c488d31 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -102,8 +102,6 @@
#include <linux/phy_fixed.h>
#include <linux/of.h>
#include <linux/of_net.h>
-#include <linux/of_address.h>
-#include <linux/of_irq.h>
#include "gianfar.h"
--
2.7.0
^ permalink raw reply related
* Re: [PATCH] net: socionext: Fix two sleep-in-atomic-context bugs in ave_rxfifo_reset()
From: David Miller @ 2018-09-18 2:03 UTC (permalink / raw)
To: baijiaju1990
Cc: hayashi.kunihiko, yamada.masahiro, robh, andrew, netdev,
linux-kernel
In-Reply-To: <20180915040246.22703-1-baijiaju1990@gmail.com>
From: Jia-Ju Bai <baijiaju1990@gmail.com>
Date: Sat, 15 Sep 2018 12:02:46 +0800
> The driver may sleep with holding a spinlock.
> The function call paths (from bottom to top) in Linux-4.17 are:
>
> [FUNC] usleep_range
> drivers/net/ethernet/socionext/sni_ave.c, 892:
> usleep_range in ave_rxfifo_reset
> drivers/net/ethernet/socionext/sni_ave.c, 932:
> ave_rxfifo_reset in ave_irq_handler
>
> [FUNC] usleep_range
> drivers/net/ethernet/socionext/sni_ave.c, 888:
> usleep_range in ave_rxfifo_reset
> drivers/net/ethernet/socionext/sni_ave.c, 932:
> ave_rxfifo_reset in ave_irq_handler
>
> To fix these bugs, usleep_range() is replaced with udelay().
>
> These bugs are found by my static analysis tool DSAC.
>
> Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
Applied to net-next.
^ permalink raw reply
* Re: [PATCH] net: bnxt: Fix a uninitialized variable warning.
From: zhong jiang @ 2018-09-18 2:00 UTC (permalink / raw)
To: Michael Chan; +Cc: Vasundhara Volam, David Miller, Netdev, open list
In-Reply-To: <CACKFLineQuBTUGaBLgRqSTEcVzyZaZAnzy0jOHREKdtCqmy6pQ@mail.gmail.com>
On 2018/9/18 1:36, Michael Chan wrote:
> On Mon, Sep 17, 2018 at 9:31 AM, zhong jiang <zhongjiang@huawei.com> wrote:
>> Fix the following compile warning:
>>
>> drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c:49:5: warning: ‘nvm_param.dir_type’ may be used uninitialized in this function [-Wmaybe-uninitialized]
>> if (nvm_param.dir_type == BNXT_NVM_PORT_CFG)
>>
>> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
>> ---
>> drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
>> index f3b9fbc..ab88217 100644
>> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
>> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
>> @@ -31,7 +31,7 @@ static int bnxt_hwrm_nvm_req(struct bnxt *bp, u32 param_id, void *msg,
>> {
>> struct hwrm_nvm_get_variable_input *req = msg;
>> void *data_addr = NULL, *buf = NULL;
>> - struct bnxt_dl_nvm_param nvm_param;
>> + struct bnxt_dl_nvm_param nvm_param = {0};
>> int bytesize, idx = 0, rc, i;
>> dma_addr_t data_dma_addr;
>>
> I think it is better to return error if there is no param_id match
> after the for loop. The for loop will initialize nvm_param if there
> is param_id match.
Will do in v2. Thanks for review.
Sincerely,
zhong jiang
^ permalink raw reply
* Re: [PATCH v2] kcm: remove any offset before parsing messages
From: Dominique Martinet @ 2018-09-18 1:57 UTC (permalink / raw)
To: David Miller; +Cc: doronrk, tom, davejwatson, netdev, linux-kernel
In-Reply-To: <20180917.184502.447385458615284933.davem@davemloft.net>
David Miller wrote on Mon, Sep 17, 2018:
> From: Dominique Martinet <asmadeus@codewreck.org>
> Date: Wed, 12 Sep 2018 07:36:42 +0200
>
> > Dominique Martinet wrote on Tue, Sep 11, 2018:
> >> Hmm, while trying to benchmark this, I sometimes got hangs in
> >> kcm_wait_data() for the last packet somehow?
> >> The sender program was done (exited (zombie) so I assumed the sender
> >> socket flushed), but the receiver was in kcm_wait_data in kcm_recvmsg
> >> indicating it parsed a header but there was no skb to peek at?
> >> But the sock is locked so this shouldn't be racy...
> >>
> >> I can get it fairly often with this patch and small messages with an
> >> offset, but I think it's just because the pull changes some timing - I
> >> can't hit it with just the clone, and I can hit it with a pull without
> >> clone as well.... And I don't see how pulling a cloned skb can impact
> >> the original socket, but I'm a bit fuzzy on this.
> >
> > This is weird, I cannot reproduce at all without that pull, even if I
> > add another delay there instead of the pull, so it's not just timing...
>
> I really can't apply this patch until you resolve this.
>
> It is weird, given your description, though...
Thanks for the reminder! I totally agree with you here and did not
expect this to be merged as it is (in retrospect, I probably should have
written something to that extent in the subject, "RFC"?)
I really don't have much time to give to that right now as I'm doing
this on my freetime, and the lack of reply has been rather demotivating
so it got pushed back a few times...
Given you did reply now I'll try to spend some time to figure that out
in the next couple of weeks but it might not make it for this cycle
depending on the number of rc we'll get and time you want this to soak
it -next.
(I can start by putting the pull back in netparser and try to reproduce,
it's really weird that I never got it to happen at the time...)
--
Dominique
^ permalink raw reply
* Re: [PATCH v2 2/2] netlink: add ethernet address policy types
From: Marcelo Ricardo Leitner @ 2018-09-17 20:26 UTC (permalink / raw)
To: Johannes Berg; +Cc: netdev, Michal Kubecek, Johannes Berg
In-Reply-To: <20180917095729.11185-2-johannes@sipsolutions.net>
On Mon, Sep 17, 2018 at 11:57:29AM +0200, Johannes Berg wrote:
> From: Johannes Berg <johannes.berg@intel.com>
>
> Commonly, ethernet addresses are just using a policy of
> { .len = ETH_ALEN }
> which leaves userspace free to send more data than it should,
> which may hide bugs.
>
> Introduce NLA_EXACT_LEN which checks for exact size, rejecting
> the attribute if it's not exactly that length. Also add
> NLA_EXACT_LEN_WARN which requires the minimum length and will
> warn on longer attributes, for backward compatibility.
>
> Use these to define NLA_POLICY_ETH_ADDR (new strict policy) and
> NLA_POLICY_ETH_ADDR_COMPAT (compatible policy with warning);
> these are used like this:
>
> static const struct nla_policy <name>[...] = {
> [NL_ATTR_NAME] = NLA_POLICY_ETH_ADDR,
> ...
> };
>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Reviewed-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> ---
> v2: add only NLA_EXACT_LEN/NLA_EXACT_LEN_WARN and build on top
> of that for ethernet address validation, so it can be extended
> for other types (e.g. IPv6 addresses)
> ---
> include/net/netlink.h | 13 +++++++++++++
> lib/nlattr.c | 8 +++++++-
> 2 files changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/include/net/netlink.h b/include/net/netlink.h
> index b318b0a9f6c3..318b1ded3833 100644
> --- a/include/net/netlink.h
> +++ b/include/net/netlink.h
> @@ -181,6 +181,8 @@ enum {
> NLA_S64,
> NLA_BITFIELD32,
> NLA_REJECT,
> + NLA_EXACT_LEN,
> + NLA_EXACT_LEN_WARN,
> __NLA_TYPE_MAX,
> };
>
> @@ -211,6 +213,10 @@ enum {
> * just like "All other"
> * NLA_BITFIELD32 Unused
> * NLA_REJECT Unused
> + * NLA_EXACT_LEN Attribute must have exactly this length, otherwise
> + * it is rejected.
> + * NLA_EXACT_LEN_WARN Attribute should have exactly this length, a warning
> + * is logged if it is longer, shorter is rejected.
> * All other Minimum length of attribute payload
> *
> * Meaning of `validation_data' field:
> @@ -236,6 +242,13 @@ struct nla_policy {
> void *validation_data;
> };
>
> +#define NLA_POLICY_EXACT_LEN(_len) { .type = NLA_EXACT_LEN, .len = _len }
> +#define NLA_POLICY_EXACT_LEN_WARN(_len) { .type = NLA_EXACT_LEN_WARN, \
> + .len = _len }
> +
> +#define NLA_POLICY_ETH_ADDR NLA_POLICY_EXACT_LEN(ETH_ALEN)
> +#define NLA_POLICY_ETH_ADDR_COMPAT NLA_POLICY_EXACT_LEN_WARN(ETH_ALEN)
> +
> /**
> * struct nl_info - netlink source information
> * @nlh: Netlink message header of original request
> diff --git a/lib/nlattr.c b/lib/nlattr.c
> index 36d74b079151..bb6fe5ed4ecf 100644
> --- a/lib/nlattr.c
> +++ b/lib/nlattr.c
> @@ -82,12 +82,18 @@ static int validate_nla(const struct nlattr *nla, int maxtype,
>
> BUG_ON(pt->type > NLA_TYPE_MAX);
>
> - if (nla_attr_len[pt->type] && attrlen != nla_attr_len[pt->type]) {
> + if ((nla_attr_len[pt->type] && attrlen != nla_attr_len[pt->type]) ||
> + (pt->type == NLA_EXACT_LEN_WARN && attrlen != pt->len)) {
> pr_warn_ratelimited("netlink: '%s': attribute type %d has an invalid length.\n",
> current->comm, type);
> }
>
> switch (pt->type) {
> + case NLA_EXACT_LEN:
> + if (attrlen != pt->len)
> + return -ERANGE;
> + break;
> +
> case NLA_REJECT:
> if (pt->validation_data && error_msg)
> *error_msg = pt->validation_data;
> --
> 2.14.4
>
^ 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