Netdev List
 help / color / mirror / Atom feed
* 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

* 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 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 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: [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] 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

* [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 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] 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] 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

* 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

* [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

* [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

* 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

* 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: [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 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: [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

* [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 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

* [GIT] Networking
From: David Miller @ 2018-09-18  3:00 UTC (permalink / raw)
  To: gregkh; +Cc: akpm, netdev, linux-kernel


Various fixes, all over the place:

1) OOB data generation fix in bluetooth, from Matias Karhumaa.

2) BPF BTF boundary calculation fix, from Martin KaFai Lau.

3) Don't bug on excessive frags, to be compatible in situations mixing
   older and newer kernels on each end.  From Juergen Gross.

4) Scheduling in RCU fix in hv_netvsc, from Stephen Hemminger.

5) Zero keying information in TLS layer before freeing copies
   of them, from Sabrina Dubroca.

6) Fix NULL deref in act_sample, from Davide Caratti.

7) Orphan SKB before GRO in veth to prevent crashes with XDP,
   from Toshiaki Makita.

8) Fix use after free in ip6_xmit, from Eric Dumazet.

9) Fix VF mac address regression in bnxt_en, from Micahel Chan.

10) Fix MSG_PEEK behavior in TLS layer, from Daniel Borkmann.

11) Programming adjustments to r8169 which fix not being to enter deep
    sleep states on some machines, from Kai-Heng Feng and Hans de
    Goede.

12) Fix DST_NOCOUNT flag handling for ipv6 routes, from Peter
    Oskolkov.

Please pull, thanks a lot!

The following changes since commit 7428b2e5d0b195f2a5e40f91d2b41a8503fcfe68:

  Merge tag 'drm-fixes-2018-09-12' of git://anongit.freedesktop.org/drm/drm (2018-09-12 17:36:47 -1000)

are available in the Git repository at:

  gitolite@ra.kernel.org:/pub/scm/linux/kernel/git/davem/net.git 

for you to fetch changes up to 30bfd93062814d6767e452a8f5ddcd97f7e38c7e:

  net/ipv6: do not copy dst flags on rt init (2018-09-17 19:42:14 -0700)

----------------------------------------------------------------
Alexei Starovoitov (1):
      bpf/verifier: disallow pointer subtraction

Andrew Lunn (1):
      net: dsa: mv88e6xxx: Fix ATU Miss Violation

Antoine Tenart (1):
      net: mvpp2: let phylink manage the carrier state

Bjørn Mork (1):
      qmi_wwan: set DTR for modems in forced USB2 mode

Colin Ian King (1):
      net: hp100: fix always-true check for link up state

Cong Wang (1):
      net_sched: notify filter deletion when deleting a chain

Corentin Labbe (1):
      net: ethernet: ti: add missing GENERIC_ALLOCATOR dependency

Daniel Borkmann (1):
      tls: fix currently broken MSG_PEEK behavior

David S. Miller (7):
      Merge branch 'tls-don-t-leave-keys-in-kernel-memory'
      Merge branch 'for-upstream' of git://git.kernel.org/.../bluetooth/bluetooth
      Merge branch 'udp-add-missing-check-on-edumx-rx-path'
      Merge git://git.kernel.org/.../bpf/bpf
      Merge branch 'hv_netvsc-associate-VF-and-PV-device-by-serial-number'
      Revert "kcm: remove any offset before parsing messages"
      Merge branch 'r8169-clk-fixes'

Davide Caratti (1):
      net/sched: act_sample: fix NULL dereference in the data path

Dominique Martinet (1):
      kcm: remove any offset before parsing messages

Eric Dumazet (1):
      ipv6: fix possible use-after-free in ip6_xmit()

Guillaume Nault (1):
      pppoe: fix reception of frames with no mac header

Hans de Goede (3):
      clk: x86: add "ether_clk" alias for Bay Trail / Cherry Trail
      r8169: Get and enable optional ether_clk clock
      clk: x86: Stop marking clocks as CLK_IS_CRITICAL

Hermes Zhang (1):
      Bluetooth: hci_ldisc: Free rw_semaphore on close

Johan Hedberg (1):
      Bluetooth: SMP: Fix trying to use non-existent local OOB data

Johannes Berg (1):
      socket: fix struct ifreq size in compat ioctl

Jongsung Kim (1):
      stmmac: fix valid numbers of unicast filter entries

Juergen Gross (1):
      xen/netfront: don't bug in case of too many frags

Kai-Heng Feng (2):
      r8169: Align ASPM/CLKREQ setting function with vendor driver
      r8169: enable ASPM on RTL8106E

Martin KaFai Lau (1):
      bpf: btf: Fix end boundary calculation for type section

Matias Karhumaa (1):
      Bluetooth: Use correct tfm to generate OOB data

Michael Chan (1):
      bnxt_en: Fix VF mac address regression.

Nicolas Ferre (2):
      net: macb: disable scatter-gather for macb on sama5d3
      ARM: dts: at91: add new compatibility string for macb on sama5d3

Paolo Abeni (2):
      udp4: fix IP_CMSG_CHECKSUM for connected sockets
      udp6: add missing checks on edumux packet processing

Peter Oskolkov (1):
      net/ipv6: do not copy dst flags on rt init

Roopa Prabhu (1):
      net: rtnl_configure_link: fix dev flags changes arg to __dev_notify_flags

Sabrina Dubroca (3):
      tls: don't copy the key out of tls12_crypto_info_aes_gcm_128
      tls: zero the crypto information from tls_context before freeing
      tls: clear key material from kernel memory when do_tls_setsockopt_conf fails

Stephen Hemminger (3):
      hv_netvsc: fix schedule in RCU context
      PCI: hv: support reporting serial number as slot information
      hv_netvsc: pair VF based on serial number

Toke Høiland-Jørgensen (1):
      gso_segment: Reset skb->mac_len after modifying network header

Toshiaki Makita (1):
      veth: Orphan skb before GRO

Tushar Dave (1):
      bpf: use __GFP_COMP while allocating page

Vasily Khoruzhick (1):
      neighbour: confirm neigh entries when ARP packet is received

Willy Tarreau (1):
      net/appletalk: fix minor pointer leak to userspace in SIOCFINDIPDDPRT

Xin Long (1):
      ipv6: use rt6_info members when dst is set in rt6_fill_node

zhong jiang (1):
      net: ethernet: Fix a unused function warning.

 Documentation/devicetree/bindings/net/macb.txt        |  1 +
 arch/arm/boot/dts/sama5d3_emac.dtsi                   |  2 +-
 drivers/bluetooth/hci_ldisc.c                         |  2 ++
 drivers/clk/x86/clk-pmc-atom.c                        | 18 +++++++++++-------
 drivers/net/appletalk/ipddp.c                         |  8 ++++++--
 drivers/net/dsa/mv88e6xxx/global1.h                   |  2 +-
 drivers/net/dsa/mv88e6xxx/global1_atu.c               |  2 +-
 drivers/net/ethernet/broadcom/bnxt/bnxt.c             |  9 +++++++--
 drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c       |  9 +++++----
 drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.h       |  2 +-
 drivers/net/ethernet/cadence/macb_main.c              |  8 ++++++++
 drivers/net/ethernet/hp/hp100.c                       |  2 +-
 drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c       | 21 ++++++---------------
 drivers/net/ethernet/microchip/lan743x_main.c         |  6 +++---
 drivers/net/ethernet/realtek/r8169.c                  | 40 +++++++++++++++++++++++++++++++++++++++-
 drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c |  5 ++---
 drivers/net/ethernet/ti/Kconfig                       |  1 +
 drivers/net/hyperv/netvsc.c                           |  3 +++
 drivers/net/hyperv/netvsc_drv.c                       | 67 ++++++++++++++++++++++++++++++++++++-------------------------------
 drivers/net/ppp/pppoe.c                               |  3 +++
 drivers/net/usb/qmi_wwan.c                            | 14 +++++++-------
 drivers/net/veth.c                                    |  4 ++--
 drivers/net/xen-netfront.c                            |  8 +++++++-
 drivers/pci/controller/pci-hyperv.c                   | 37 +++++++++++++++++++++++++++++++++++++
 include/net/tls.h                                     | 19 +++++++++----------
 kernel/bpf/btf.c                                      |  2 +-
 kernel/bpf/verifier.c                                 |  2 +-
 net/bluetooth/smp.c                                   | 16 +++++++++++++---
 net/core/filter.c                                     |  3 ++-
 net/core/neighbour.c                                  | 13 ++++++++-----
 net/core/rtnetlink.c                                  |  2 +-
 net/ipv4/af_inet.c                                    |  1 +
 net/ipv4/udp.c                                        | 49 ++++++++++++++++++++++++++-----------------------
 net/ipv6/ip6_offload.c                                |  1 +
 net/ipv6/ip6_output.c                                 |  6 ++----
 net/ipv6/route.c                                      | 44 ++++++++++++++++++++++++++++++--------------
 net/ipv6/udp.c                                        | 65 +++++++++++++++++++++++++++++++++++++----------------------------
 net/sched/act_sample.c                                |  2 +-
 net/sched/cls_api.c                                   |  2 ++
 net/socket.c                                          | 22 ++++++++++++++--------
 net/tls/tls_device.c                                  |  6 +++---
 net/tls/tls_device_fallback.c                         |  2 +-
 net/tls/tls_main.c                                    | 22 ++++++++++++++++------
 net/tls/tls_sw.c                                      | 21 +++++++++++++--------
 tools/testing/selftests/net/tls.c                     | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
 45 files changed, 422 insertions(+), 201 deletions(-)

^ permalink raw reply

* KMSAN: uninit-value in __dev_mc_del
From: syzbot @ 2018-09-17 21:33 UTC (permalink / raw)
  To: davem, edumazet, linux-kernel, netdev, sunlw.fnst, syzkaller-bugs

Hello,

syzbot found the following crash on:

HEAD commit:    5815ca49bf07 Temporarily revert "Revert "kmsan: don't trac..
git tree:       https://github.com/google/kmsan.git/master
console output: https://syzkaller.appspot.com/x/log.txt?x=14dafb66400000
kernel config:  https://syzkaller.appspot.com/x/.config?x=6be230b6f0ad3490
dashboard link: https://syzkaller.appspot.com/bug?extid=0ffee94c5c059dbbc2a9
compiler:       clang version 8.0.0 (trunk 339414)
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=1417c62a400000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=17ce684e400000

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+0ffee94c5c059dbbc2a9@syzkaller.appspotmail.com

IPv6: ADDRCONF(NETDEV_CHANGE): veth0: link becomes ready
8021q: adding VLAN 0 to HW filter on device team0
IPv6: ADDRCONF(NETDEV_CHANGE): vcan0: link becomes ready
IPv6: ADDRCONF(NETDEV_CHANGE): vcan0: link becomes ready
==================================================================
BUG: KMSAN: uninit-value in memcmp+0x11d/0x180 lib/string.c:863
CPU: 1 PID: 4510 Comm: syz-executor267 Not tainted 4.19.0-rc3+ #48
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+0x14b/0x190 lib/dump_stack.c:113
  kmsan_report+0x183/0x2b0 mm/kmsan/kmsan.c:956
  __msan_warning+0x70/0xc0 mm/kmsan/kmsan_instr.c:645
  memcmp+0x11d/0x180 lib/string.c:863
  __hw_addr_del_ex net/core/dev_addr_lists.c:123 [inline]
  __dev_mc_del+0x169/0x6c0 net/core/dev_addr_lists.c:710
  dev_mc_del+0x6d/0x80 net/core/dev_addr_lists.c:728
  ip_mc_filter_del net/ipv4/igmp.c:1129 [inline]
  igmp_group_dropped+0x21a/0x1280 net/ipv4/igmp.c:1262
  ip_mc_down+0x1d9/0x3e0 net/ipv4/igmp.c:1712
  inetdev_event+0x238/0x1d70 net/ipv4/devinet.c:1522
  notifier_call_chain kernel/notifier.c:93 [inline]
  __raw_notifier_call_chain kernel/notifier.c:394 [inline]
  raw_notifier_call_chain+0x13e/0x240 kernel/notifier.c:401
  call_netdevice_notifiers_info net/core/dev.c:1733 [inline]
  call_netdevice_notifiers net/core/dev.c:1751 [inline]
  dev_close_many+0x627/0x9e0 net/core/dev.c:1503
  rollback_registered_many+0x9eb/0x2040 net/core/dev.c:7929
  rollback_registered net/core/dev.c:7994 [inline]
  unregister_netdevice_queue+0x547/0xa40 net/core/dev.c:9038
  unregister_netdevice include/linux/netdevice.h:2589 [inline]
  __tun_detach+0x220c/0x2c70 drivers/net/tun.c:729
  tun_detach drivers/net/tun.c:746 [inline]
  tun_chr_close+0xda/0x1c0 drivers/net/tun.c:3272
  __fput+0x4cf/0xc20 fs/file_table.c:278
  ____fput+0x37/0x40 fs/file_table.c:309
  task_work_run+0x22e/0x2b0 kernel/task_work.c:113
  tracehook_notify_resume include/linux/tracehook.h:193 [inline]
  exit_to_usermode_loop arch/x86/entry/common.c:166 [inline]
  prepare_exit_to_usermode+0x33e/0x410 arch/x86/entry/common.c:197
  syscall_return_slowpath+0xdb/0x700 arch/x86/entry/common.c:268
  do_syscall_64+0xde/0x100 arch/x86/entry/common.c:294
  entry_SYSCALL_64_after_hwframe+0x63/0xe7
RIP: 0033:0x401540
Code: 01 f0 ff ff 0f 83 b0 0a 00 00 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f  
44 00 00 83 3d ed 22 2d 00 00 75 14 b8 03 00 00 00 0f 05 <48> 3d 01 f0 ff  
ff 0f 83 84 0a 00 00 c3 48 83 ec 08 e8 3a 01 00 00
RSP: 002b:00007fffbb917ce8 EFLAGS: 00000246 ORIG_RAX: 0000000000000003
RAX: 0000000000000000 RBX: 0000000000000003 RCX: 0000000000401540
RDX: 0000000000000000 RSI: 0000000020000100 RDI: 0000000000000003
RBP: 00000000006cc018 R08: 0000000000000100 R09: 0000000000000100
R10: 0000000000000100 R11: 0000000000000246 R12: 0000000000000000
R13: 00000000004024e0 R14: 0000000000000000 R15: 0000000000000000

Local variable description: ----buf.i@igmp_group_dropped
Variable was created at:
  igmp_group_dropped+0x49/0x1280 net/ipv4/igmp.c:1253
  ip_mc_down+0x1d9/0x3e0 net/ipv4/igmp.c:1712
==================================================================


---
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.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches

^ permalink raw reply

* [PATCH net-next] net: ethernet: slicoss: remove duplicated include from slic.h
From: YueHaibing @ 2018-09-18  3:09 UTC (permalink / raw)
  To: davem, LinoSanfilippo; +Cc: linux-kernel, netdev, YueHaibing

Remove duplicated include.

Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 drivers/net/ethernet/alacritech/slic.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/alacritech/slic.h b/drivers/net/ethernet/alacritech/slic.h
index d0c388c..3add305 100644
--- a/drivers/net/ethernet/alacritech/slic.h
+++ b/drivers/net/ethernet/alacritech/slic.h
@@ -8,7 +8,6 @@
 #include <linux/spinlock_types.h>
 #include <linux/dma-mapping.h>
 #include <linux/pci.h>
-#include <linux/netdevice.h>
 #include <linux/list.h>
 #include <linux/u64_stats_sync.h>
 
-- 
2.7.0

^ permalink raw reply related

* Re: [PATCH 0/2] add Ethernet driver support for mt2712
From: biao huang @ 2018-09-18  3:24 UTC (permalink / raw)
  To: Jose Abreu, Andrew Lunn
  Cc: peppe.cavallaro, alexandre.torgue, davem, robh+dt, honghui.zhang,
	yt.shen, liguo.zhang, mark.rutland, sean.wang, nelson.chang,
	matthias.bgg, netdev, devicetree, linux-kernel, linux-arm-kernel,
	linux-mediatek
In-Reply-To: <a8c0a091-6bee-6504-04cd-36e214e805aa@synopsys.com>

Hi Jose, Andrew,
	Thanks for your comments.
	Synopsys ip version in mt2712 is 4.21a, and followed ic will use 5.10a.
it seems GMAC4+ is a good choice. I'll try to  extend STMMAC to support
mt2712.
	Any tips about extend STMMAC? or anythings I should pay attention to?
	
On Mon, 2018-09-17 at 17:18 +0100, Jose Abreu wrote:
> Hi Andrew, Biao,
> 
> On 17-09-2018 16:24, Andrew Lunn wrote:
> > On Mon, Sep 17, 2018 at 02:29:21PM +0800, Biao Huang wrote:
> >
> > Adding in the STMMAC driver maintainers.
> >
> >> Ethernet in mt2712 is totally different from that in
> >> drivers/net/ethernet/mediatek/*, so we add new folder for mt2712 SoC.
> >>
> >> The mt2712 Ethernet IP is from Synopsys, and we notice that there is a
> >> reference driver in drivers/net/ethernet/synopsys/*. But
> >> 1. our version is only for 10/100/1000Mbps, not for 2.5/4/5Gbps.
> >> mt2712 Ethernet design is differnet from that in synopsys folder in many
> >> aspects, and some key features are not included in mt2712, such as rss
> >> and split header. At the same time, some features we need have not been
> >> implenmented in synopsys folder.
> > In general, we don't have two very similar drivers. We try to have one
> > driver. If the problem was just missing features in the stmmac driver,
> > you can add them. I doubt not supporting 2.5/4/5Gbps in your silicon
> > is an issue, since very few STMMAC devices have this. By split header,
> > do you mean support for TSO? That seems to be a gmac4 or newer
> > feature, but the driver supports not having tso support in hardware.
> >
> > Giuseppe, Alexandre, Jose: Please can you look at the proposed driver
> > and see how much it really differs from the STMMAC driver. 
> 
> Thanks for the cc Andrew, indeed this looks very similar and the
> register bank matches, by what I've seen, GMAC 4+.
> 
> > How easy
> > would it be to extend stmmac it to support the mt2712?
> 
> Very easy, as I've just done with XGMAC2. If Biao wants to expand
> stmmac functionality I'm all in favor!
> 
> Thanks and Best Regards,
> Jose Miguel Abreu
> 
> >
> > Thanks
> > 	Andrew
> 
Best Regards!
Biao

^ permalink raw reply

* Re: [PATCH bpf-next] tools/bpf: bpftool: improve output format for bpftool net
From: Yonghong Song @ 2018-09-17 22:21 UTC (permalink / raw)
  To: Daniel Borkmann, Alexei Starovoitov, netdev@vger.kernel.org; +Cc: Kernel Team
In-Reply-To: <5e65458e-aec5-baf7-9e0f-50e3647542ff@iogearbox.net>



On 9/17/18 3:19 AM, Daniel Borkmann wrote:
> On 09/14/2018 11:49 PM, Yonghong Song wrote:
>> This is a followup patch for Commit f6f3bac08ff9
>> ("tools/bpf: bpftool: add net support").
>> Some improvements are made for the bpftool net output.
>> Specially, plain output is more concise such that
>> per attachment should nicely fit in one line.
>> Compared to previous output, the prog tag is removed
>> since it can be easily obtained with program id.
>> Similar to xdp attachments, the device name is added
>> to tc_filters attachments.
>>
>> The bpf program attached through shared block
>> mechanism is supported as well.
>>    $ ip link add dev v1 type veth peer name v2
>>    $ tc qdisc add dev v1 ingress_block 10 egress_block 20 clsact
>>    $ tc qdisc add dev v2 ingress_block 10 egress_block 20 clsact
>>    $ tc filter add block 10 protocol ip prio 25 bpf obj bpf_shared.o sec ingress flowid 1:1
>>    $ tc filter add block 20 protocol ip prio 30 bpf obj bpf_cyclic.o sec classifier flowid 1:1
>>    $ bpftool net
>>    xdp [
>>    ]
>>    tc_filters [
>>     v2(7) qdisc_clsact_ingress bpf_shared.o:[ingress] id 23
>>     v2(7) qdisc_clsact_egress bpf_cyclic.o:[classifier] id 24
>>     v1(8) qdisc_clsact_ingress bpf_shared.o:[ingress] id 23
>>     v1(8) qdisc_clsact_egress bpf_cyclic.o:[classifier] id 24
> 
> Just one minor note for this one here, do we even need the "qdisc_" prefix? Couldn't it just simply
> be "clsact/ingress", "clsact/egress", "htb" etc?

Will do.

> 
>>    ]
>>
>> The documentation and "bpftool net help" are updated
>> to make it clear that current implementation only
>> supports xdp and tc attachments. For programs
>> attached to cgroups, "bpftool cgroup" can be used
>> to dump attachments. For other programs e.g.
>> sk_{filter,skb,msg,reuseport} and lwt/seg6,
>> iproute2 tools should be used.
>>
>> The new output:
>>    $ bpftool net
>>    xdp [
>>     eth0(2) id/drv 198
> 
> Could we change the "id/{drv,offload,generic} xyz" into e.g. "eth0(2) {driver,offload,generic} id 198",
> meaning, the "id xyz" being a child of either "driver", "offload" or "generic". Reason would be two-fold:
> i) we can keep the "id xyz" notion consistent as used under "tc_filters", and ii) it allows to put further
> information aside from just "id" member under "driver", "offload" or "generic" in the future.

Will do.

> 
>>    ]
>>    tc_filters [
> 
> Nit: can we use just "tc" for the above? Main use case would be clsact with one of its two hooks anyway,
> and the term "filter" is sort of tc historic; while being correct bpf progs would do much more than just
> filtering, and context is pretty clear anyway from qdisc that we subsequently dump.

Make sense.

Will address all these comments and submit a revision soon. Thanks!

> 
>>     eth0(2) qdisc_clsact_ingress fbflow_icmp id 335 act [{icmp_action id 336}]
>>     eth0(2) qdisc_clsact_egress fbflow_egress id 334
>>    ]
>>    $ bpftool -jp net
>>    [{
>>          "xdp": [{
>>                  "devname": "eth0",
>>                  "ifindex": 2,
>>                  "id/drv": 198
>>              }
>>          ],
>>          "tc_filters": [{
>>                  "devname": "eth0",
>>                  "ifindex": 2,
>>                  "kind": "qdisc_clsact_ingress",
>>                  "name": "fbflow_icmp",
>>                  "id": 335,
>>                  "act": [{
>>                          "name": "icmp_action",
>>                          "id": 336
>>                      }
>>                  ]
>>              },{
>>                  "devname": "eth0",
>>                  "ifindex": 2,
>>                  "kind": "qdisc_clsact_egress",
>>                  "name": "fbflow_egress",
>>                  "id": 334
>>              }
>>          ]
>>      }
>>    ]
>>
>> Signed-off-by: Yonghong Song <yhs@fb.com>

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox