* Re: [net-next 2/4] gve: Add transmit and receive support
From: Catherine Sullivan @ 2019-06-28 17:48 UTC (permalink / raw)
To: David Miller
Cc: netdev, Sagi Shahar, Jon Olson, Willem de Bruijn, Luigi Rizzo
In-Reply-To: <20190626.124917.1144578915345631665.davem@davemloft.net>
On Wed, Jun 26, 2019 at 12:49 PM David Miller <davem@davemloft.net> wrote:
>
> From: Catherine Sullivan <csully@google.com>
> Date: Wed, 26 Jun 2019 11:52:49 -0700
>
> > +#ifdef __LITTLE_ENDIAN
> > +#define GVE_SEQNO(x) ((((__force u16)x) >> 8) & 0x7)
> > +#else
> > +#define GVE_SEQNO(x) ((__force u16)(x) & 0x7)
> > +#endif
>
> This can be simply "le16_to_cpu(x) & 0x7" or similar. No need to
> messy ifdefs.
>
Will fix in v2.
^ permalink raw reply
* Re: [PATCH v3 bpf-next 2/9] libbpf: introduce concept of bpf_link
From: Stanislav Fomichev @ 2019-06-28 17:45 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Andrii Nakryiko, Alexei Starovoitov, Daniel Borkmann, Networking,
bpf, Kernel Team
In-Reply-To: <CAEf4BzbB6G5jTvS+K0+0zPXWLFmAePHU2RtALogWrh7h7OV03A@mail.gmail.com>
On 06/28, Andrii Nakryiko wrote:
> On Fri, Jun 28, 2019 at 9:02 AM Stanislav Fomichev <sdf@fomichev.me> wrote:
> >
> > On 06/27, Andrii Nakryiko wrote:
> > > bpf_link is and abstraction of an association of a BPF program and one
> > > of many possible BPF attachment points (hooks). This allows to have
> > > uniform interface for detaching BPF programs regardless of the nature of
> > > link and how it was created. Details of creation and setting up of
> > > a specific bpf_link is handled by corresponding attachment methods
> > > (bpf_program__attach_xxx) added in subsequent commits. Once successfully
> > > created, bpf_link has to be eventually destroyed with
> > > bpf_link__destroy(), at which point BPF program is disassociated from
> > > a hook and all the relevant resources are freed.
> > >
> > > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> > > ---
> > > tools/lib/bpf/libbpf.c | 17 +++++++++++++++++
> > > tools/lib/bpf/libbpf.h | 4 ++++
> > > tools/lib/bpf/libbpf.map | 3 ++-
> > > 3 files changed, 23 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > > index 6e6ebef11ba3..455795e6f8af 100644
> > > --- a/tools/lib/bpf/libbpf.c
> > > +++ b/tools/lib/bpf/libbpf.c
> > > @@ -3941,6 +3941,23 @@ int bpf_prog_load_xattr(const struct bpf_prog_load_attr *attr,
> > > return 0;
> > > }
> > >
> > > +struct bpf_link {
> > Maybe call it bpf_attachment? You call the bpf_program__attach_to_blah
> > and you get an attachment?
>
> I wanted to keep it as short as possible, bpf_attachment is way too
> long (it's also why as an alternative I've proposed bpf_assoc, not
> bpf_association, but bpf_attach isn't great shortening).
Why do you want to keep it short? We have far longer names than
bpf_attachment in libbpf. That shouldn't be a big concern.
> > > + int (*destroy)(struct bpf_link *link);
> > > +};
> > > +
> > > +int bpf_link__destroy(struct bpf_link *link)
> > > +{
> > > + int err;
> > > +
> > > + if (!link)
> > > + return 0;
> > > +
> > > + err = link->destroy(link);
> > > + free(link);
> > > +
> > > + return err;
> > > +}
> > > +
> > > enum bpf_perf_event_ret
> > > bpf_perf_event_read_simple(void *mmap_mem, size_t mmap_size, size_t page_size,
> > > void **copy_mem, size_t *copy_size,
> > > diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> > > index d639f47e3110..5082a5ebb0c2 100644
> > > --- a/tools/lib/bpf/libbpf.h
> > > +++ b/tools/lib/bpf/libbpf.h
> > > @@ -165,6 +165,10 @@ LIBBPF_API int bpf_program__pin(struct bpf_program *prog, const char *path);
> > > LIBBPF_API int bpf_program__unpin(struct bpf_program *prog, const char *path);
> > > LIBBPF_API void bpf_program__unload(struct bpf_program *prog);
> > >
> > > +struct bpf_link;
> > > +
> > > +LIBBPF_API int bpf_link__destroy(struct bpf_link *link);
> > > +
> > > struct bpf_insn;
> > >
> > > /*
> > > diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> > > index 2c6d835620d2..3cde850fc8da 100644
> > > --- a/tools/lib/bpf/libbpf.map
> > > +++ b/tools/lib/bpf/libbpf.map
> > > @@ -167,10 +167,11 @@ LIBBPF_0.0.3 {
> > >
> > > LIBBPF_0.0.4 {
> > > global:
> > > + bpf_link__destroy;
> > > + bpf_object__load_xattr;
> > > btf_dump__dump_type;
> > > btf_dump__free;
> > > btf_dump__new;
> > > btf__parse_elf;
> > > - bpf_object__load_xattr;
> > > libbpf_num_possible_cpus;
> > > } LIBBPF_0.0.3;
> > > --
> > > 2.17.1
> > >
^ permalink raw reply
* Re: [net-next 4/4] gve: Add ethtool support
From: Catherine Sullivan @ 2019-06-28 17:42 UTC (permalink / raw)
To: Andrew Lunn; +Cc: netdev, Sagi Shahar, Jon Olson, Willem de Bruijn, Luigi Rizzo
In-Reply-To: <20190626194805.GG27733@lunn.ch>
On Wed, Jun 26, 2019 at 12:48 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > +static int gve_get_sset_count(struct net_device *netdev, int sset)
> > +{
> > + struct gve_priv *priv = netdev_priv(netdev);
> > +
> > + if (!netif_carrier_ok(netdev))
> > + return 0;
>
> That is pretty unusual. What goes wrong if there is no carrier and
> statistics are returned?
This was meant to protect against priv->rx/tx not being a valid pointer.
In v2 it will just check the pointer instead of the carrier status.
>
> > +static void
> > +gve_get_ethtool_stats(struct net_device *netdev,
> > + struct ethtool_stats *stats, u64 *data)
> > +{
> > + struct gve_priv *priv = netdev_priv(netdev);
> > + u64 rx_pkts, rx_bytes, tx_pkts, tx_bytes;
> > + int ring;
> > + int i;
> > +
> > + ASSERT_RTNL();
> > +
> > + if (!netif_carrier_ok(netdev))
> > + return;
> > +
> > + for (rx_pkts = 0, rx_bytes = 0, ring = 0;
> > + ring < priv->rx_cfg.num_queues; ring++) {
> > + rx_pkts += priv->rx[ring].rpackets;
> > + rx_bytes += priv->rx[ring].rbytes;
> > + }
> > + for (tx_pkts = 0, tx_bytes = 0, ring = 0;
> > + ring < priv->tx_cfg.num_queues; ring++) {
> > + tx_pkts += priv->tx[ring].pkt_done;
> > + tx_bytes += priv->tx[ring].bytes_done;
> > + }
> > + memset(data, 0, GVE_MAIN_STATS_LEN * sizeof(*data));
>
> Maybe you should do this memset when the carrier is off?
Will be fixed in v2.
>
> Andrew
Catherine
^ permalink raw reply
* [PATCH 0/4] Netfilter/IPVS fixes for net
From: Pablo Neira Ayuso @ 2019-06-28 17:41 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
Hi,
The following patchset contains Netfilter fixes for net:
1) Fix memleak reported by syzkaller when registering IPVS hooks,
patch from Julian Anastasov.
2) Fix memory leak in start_sync_thread, also from Julian.
3) Fix conntrack deletion via ctnetlink, from Felix Kaechele.
4) Fix reject for ICMP due to incorrect checksum handling, from
He Zhe.
You can pull these changes from:
git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git
Thanks.
----------------------------------------------------------------
The following changes since commit 85f9aa7565bd79b039325f2c01af7ffa717924df:
inet: clear num_timeout reqsk_alloc() (2019-06-19 17:46:57 -0400)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git HEAD
for you to fetch changes up to 5d1549847c76b1ffcf8e388ef4d0f229bdd1d7e8:
netfilter: Fix remainder of pseudo-header protocol 0 (2019-06-28 19:30:50 +0200)
----------------------------------------------------------------
Felix Kaechele (1):
netfilter: ctnetlink: Fix regression in conntrack entry deletion
He Zhe (1):
netfilter: Fix remainder of pseudo-header protocol 0
Julian Anastasov (2):
ipvs: defer hook registration to avoid leaks
ipvs: fix tinfo memory leak in start_sync_thread
include/net/ip_vs.h | 6 +-
net/netfilter/ipvs/ip_vs_core.c | 21 +++--
net/netfilter/ipvs/ip_vs_ctl.c | 4 -
net/netfilter/ipvs/ip_vs_sync.c | 134 +++++++++++++++++---------------
net/netfilter/nf_conntrack_netlink.c | 7 +-
net/netfilter/nf_conntrack_proto_icmp.c | 2 +-
net/netfilter/nf_nat_proto.c | 2 +-
net/netfilter/utils.c | 5 +-
8 files changed, 99 insertions(+), 82 deletions(-)
^ permalink raw reply
* [PATCH 3/4] netfilter: ctnetlink: Fix regression in conntrack entry deletion
From: Pablo Neira Ayuso @ 2019-06-28 17:41 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20190628174125.20739-1-pablo@netfilter.org>
From: Felix Kaechele <felix@kaechele.ca>
Commit f8e608982022 ("netfilter: ctnetlink: Resolve conntrack
L3-protocol flush regression") introduced a regression in which deletion
of conntrack entries would fail because the L3 protocol information
is replaced by AF_UNSPEC. As a result the search for the entry to be
deleted would turn up empty due to the tuple used to perform the search
is now different from the tuple used to initially set up the entry.
For flushing the conntrack table we do however want to keep the option
for nfgenmsg->version to have a non-zero value to allow for newer
user-space tools to request treatment under the new behavior. With that
it is possible to independently flush tables for a defined L3 protocol.
This was introduced with the enhancements in in commit 59c08c69c278
("netfilter: ctnetlink: Support L3 protocol-filter on flush").
Older user-space tools will retain the behavior of flushing all tables
regardless of defined L3 protocol.
Fixes: f8e608982022 ("netfilter: ctnetlink: Resolve conntrack L3-protocol flush regression")
Suggested-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Felix Kaechele <felix@kaechele.ca>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/netfilter/nf_conntrack_netlink.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index 7db79c1b8084..1b77444d5b52 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -1256,7 +1256,6 @@ static int ctnetlink_del_conntrack(struct net *net, struct sock *ctnl,
struct nf_conntrack_tuple tuple;
struct nf_conn *ct;
struct nfgenmsg *nfmsg = nlmsg_data(nlh);
- u_int8_t u3 = nfmsg->version ? nfmsg->nfgen_family : AF_UNSPEC;
struct nf_conntrack_zone zone;
int err;
@@ -1266,11 +1265,13 @@ static int ctnetlink_del_conntrack(struct net *net, struct sock *ctnl,
if (cda[CTA_TUPLE_ORIG])
err = ctnetlink_parse_tuple(cda, &tuple, CTA_TUPLE_ORIG,
- u3, &zone);
+ nfmsg->nfgen_family, &zone);
else if (cda[CTA_TUPLE_REPLY])
err = ctnetlink_parse_tuple(cda, &tuple, CTA_TUPLE_REPLY,
- u3, &zone);
+ nfmsg->nfgen_family, &zone);
else {
+ u_int8_t u3 = nfmsg->version ? nfmsg->nfgen_family : AF_UNSPEC;
+
return ctnetlink_flush_conntrack(net, cda,
NETLINK_CB(skb).portid,
nlmsg_report(nlh), u3);
--
2.11.0
^ permalink raw reply related
* [PATCH 1/4] ipvs: defer hook registration to avoid leaks
From: Pablo Neira Ayuso @ 2019-06-28 17:41 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20190628174125.20739-1-pablo@netfilter.org>
From: Julian Anastasov <ja@ssi.bg>
syzkaller reports for memory leak when registering hooks [1]
As we moved the nf_unregister_net_hooks() call into
__ip_vs_dev_cleanup(), defer the nf_register_net_hooks()
call, so that hooks are allocated and freed from same
pernet_operations (ipvs_core_dev_ops).
[1]
BUG: memory leak
unreferenced object 0xffff88810acd8a80 (size 96):
comm "syz-executor073", pid 7254, jiffies 4294950560 (age 22.250s)
hex dump (first 32 bytes):
02 00 00 00 00 00 00 00 50 8b bb 82 ff ff ff ff ........P.......
00 00 00 00 00 00 00 00 00 77 bb 82 ff ff ff ff .........w......
backtrace:
[<0000000013db61f1>] kmemleak_alloc_recursive include/linux/kmemleak.h:55 [inline]
[<0000000013db61f1>] slab_post_alloc_hook mm/slab.h:439 [inline]
[<0000000013db61f1>] slab_alloc_node mm/slab.c:3269 [inline]
[<0000000013db61f1>] kmem_cache_alloc_node_trace+0x15b/0x2a0 mm/slab.c:3597
[<000000001a27307d>] __do_kmalloc_node mm/slab.c:3619 [inline]
[<000000001a27307d>] __kmalloc_node+0x38/0x50 mm/slab.c:3627
[<0000000025054add>] kmalloc_node include/linux/slab.h:590 [inline]
[<0000000025054add>] kvmalloc_node+0x4a/0xd0 mm/util.c:431
[<0000000050d1bc00>] kvmalloc include/linux/mm.h:637 [inline]
[<0000000050d1bc00>] kvzalloc include/linux/mm.h:645 [inline]
[<0000000050d1bc00>] allocate_hook_entries_size+0x3b/0x60 net/netfilter/core.c:61
[<00000000e8abe142>] nf_hook_entries_grow+0xae/0x270 net/netfilter/core.c:128
[<000000004b94797c>] __nf_register_net_hook+0x9a/0x170 net/netfilter/core.c:337
[<00000000d1545cbc>] nf_register_net_hook+0x34/0xc0 net/netfilter/core.c:464
[<00000000876c9b55>] nf_register_net_hooks+0x53/0xc0 net/netfilter/core.c:480
[<000000002ea868e0>] __ip_vs_init+0xe8/0x170 net/netfilter/ipvs/ip_vs_core.c:2280
[<000000002eb2d451>] ops_init+0x4c/0x140 net/core/net_namespace.c:130
[<000000000284ec48>] setup_net+0xde/0x230 net/core/net_namespace.c:316
[<00000000a70600fa>] copy_net_ns+0xf0/0x1e0 net/core/net_namespace.c:439
[<00000000ff26c15e>] create_new_namespaces+0x141/0x2a0 kernel/nsproxy.c:107
[<00000000b103dc79>] copy_namespaces+0xa1/0xe0 kernel/nsproxy.c:165
[<000000007cc008a2>] copy_process.part.0+0x11fd/0x2150 kernel/fork.c:2035
[<00000000c344af7c>] copy_process kernel/fork.c:1800 [inline]
[<00000000c344af7c>] _do_fork+0x121/0x4f0 kernel/fork.c:2369
Reported-by: syzbot+722da59ccb264bc19910@syzkaller.appspotmail.com
Fixes: 719c7d563c17 ("ipvs: Fix use-after-free in ip_vs_in")
Signed-off-by: Julian Anastasov <ja@ssi.bg>
Acked-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/netfilter/ipvs/ip_vs_core.c | 21 ++++++++++++++-------
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
index 7138556b206b..d5103a9eb302 100644
--- a/net/netfilter/ipvs/ip_vs_core.c
+++ b/net/netfilter/ipvs/ip_vs_core.c
@@ -2245,7 +2245,6 @@ static const struct nf_hook_ops ip_vs_ops[] = {
static int __net_init __ip_vs_init(struct net *net)
{
struct netns_ipvs *ipvs;
- int ret;
ipvs = net_generic(net, ip_vs_net_id);
if (ipvs == NULL)
@@ -2277,17 +2276,11 @@ static int __net_init __ip_vs_init(struct net *net)
if (ip_vs_sync_net_init(ipvs) < 0)
goto sync_fail;
- ret = nf_register_net_hooks(net, ip_vs_ops, ARRAY_SIZE(ip_vs_ops));
- if (ret < 0)
- goto hook_fail;
-
return 0;
/*
* Error handling
*/
-hook_fail:
- ip_vs_sync_net_cleanup(ipvs);
sync_fail:
ip_vs_conn_net_cleanup(ipvs);
conn_fail:
@@ -2317,6 +2310,19 @@ static void __net_exit __ip_vs_cleanup(struct net *net)
net->ipvs = NULL;
}
+static int __net_init __ip_vs_dev_init(struct net *net)
+{
+ int ret;
+
+ ret = nf_register_net_hooks(net, ip_vs_ops, ARRAY_SIZE(ip_vs_ops));
+ if (ret < 0)
+ goto hook_fail;
+ return 0;
+
+hook_fail:
+ return ret;
+}
+
static void __net_exit __ip_vs_dev_cleanup(struct net *net)
{
struct netns_ipvs *ipvs = net_ipvs(net);
@@ -2336,6 +2342,7 @@ static struct pernet_operations ipvs_core_ops = {
};
static struct pernet_operations ipvs_core_dev_ops = {
+ .init = __ip_vs_dev_init,
.exit = __ip_vs_dev_cleanup,
};
--
2.11.0
^ permalink raw reply related
* [PATCH 4/4] netfilter: Fix remainder of pseudo-header protocol 0
From: Pablo Neira Ayuso @ 2019-06-28 17:41 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20190628174125.20739-1-pablo@netfilter.org>
From: He Zhe <zhe.he@windriver.com>
Since v5.1-rc1, some types of packets do not get unreachable reply with the
following iptables setting. Fox example,
$ iptables -A INPUT -p icmp --icmp-type 8 -j REJECT
$ ping 127.0.0.1 -c 1
PING 127.0.0.1 (127.0.0.1) 56(84) bytes of data.
— 127.0.0.1 ping statistics —
1 packets transmitted, 0 received, 100% packet loss, time 0ms
We should have got the following reply from command line, but we did not.
From 127.0.0.1 icmp_seq=1 Destination Port Unreachable
Yi Zhao reported it and narrowed it down to:
7fc38225363d ("netfilter: reject: skip csum verification for protocols that don't support it"),
This is because nf_ip_checksum still expects pseudo-header protocol type 0 for
packets that are of neither TCP or UDP, and thus ICMP packets are mistakenly
treated as TCP/UDP.
This patch corrects the conditions in nf_ip_checksum and all other places that
still call it with protocol 0.
Fixes: 7fc38225363d ("netfilter: reject: skip csum verification for protocols that don't support it")
Reported-by: Yi Zhao <yi.zhao@windriver.com>
Signed-off-by: He Zhe <zhe.he@windriver.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/netfilter/nf_conntrack_proto_icmp.c | 2 +-
net/netfilter/nf_nat_proto.c | 2 +-
net/netfilter/utils.c | 5 +++--
3 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/net/netfilter/nf_conntrack_proto_icmp.c b/net/netfilter/nf_conntrack_proto_icmp.c
index 9becac953587..71a84a0517f3 100644
--- a/net/netfilter/nf_conntrack_proto_icmp.c
+++ b/net/netfilter/nf_conntrack_proto_icmp.c
@@ -221,7 +221,7 @@ int nf_conntrack_icmpv4_error(struct nf_conn *tmpl,
/* See ip_conntrack_proto_tcp.c */
if (state->net->ct.sysctl_checksum &&
state->hook == NF_INET_PRE_ROUTING &&
- nf_ip_checksum(skb, state->hook, dataoff, 0)) {
+ nf_ip_checksum(skb, state->hook, dataoff, IPPROTO_ICMP)) {
icmp_error_log(skb, state, "bad hw icmp checksum");
return -NF_ACCEPT;
}
diff --git a/net/netfilter/nf_nat_proto.c b/net/netfilter/nf_nat_proto.c
index 84f5c90a7f21..9f3e52ebd3b8 100644
--- a/net/netfilter/nf_nat_proto.c
+++ b/net/netfilter/nf_nat_proto.c
@@ -567,7 +567,7 @@ int nf_nat_icmp_reply_translation(struct sk_buff *skb,
if (!skb_make_writable(skb, hdrlen + sizeof(*inside)))
return 0;
- if (nf_ip_checksum(skb, hooknum, hdrlen, 0))
+ if (nf_ip_checksum(skb, hooknum, hdrlen, IPPROTO_ICMP))
return 0;
inside = (void *)skb->data + hdrlen;
diff --git a/net/netfilter/utils.c b/net/netfilter/utils.c
index 06dc55590441..51b454d8fa9c 100644
--- a/net/netfilter/utils.c
+++ b/net/netfilter/utils.c
@@ -17,7 +17,8 @@ __sum16 nf_ip_checksum(struct sk_buff *skb, unsigned int hook,
case CHECKSUM_COMPLETE:
if (hook != NF_INET_PRE_ROUTING && hook != NF_INET_LOCAL_IN)
break;
- if ((protocol == 0 && !csum_fold(skb->csum)) ||
+ if ((protocol != IPPROTO_TCP && protocol != IPPROTO_UDP &&
+ !csum_fold(skb->csum)) ||
!csum_tcpudp_magic(iph->saddr, iph->daddr,
skb->len - dataoff, protocol,
skb->csum)) {
@@ -26,7 +27,7 @@ __sum16 nf_ip_checksum(struct sk_buff *skb, unsigned int hook,
}
/* fall through */
case CHECKSUM_NONE:
- if (protocol == 0)
+ if (protocol != IPPROTO_TCP && protocol != IPPROTO_UDP)
skb->csum = 0;
else
skb->csum = csum_tcpudp_nofold(iph->saddr, iph->daddr,
--
2.11.0
^ permalink raw reply related
* [PATCH 2/4] ipvs: fix tinfo memory leak in start_sync_thread
From: Pablo Neira Ayuso @ 2019-06-28 17:41 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20190628174125.20739-1-pablo@netfilter.org>
From: Julian Anastasov <ja@ssi.bg>
syzkaller reports for memory leak in start_sync_thread [1]
As Eric points out, kthread may start and stop before the
threadfn function is called, so there is no chance the
data (tinfo in our case) to be released in thread.
Fix this by releasing tinfo in the controlling code instead.
[1]
BUG: memory leak
unreferenced object 0xffff8881206bf700 (size 32):
comm "syz-executor761", pid 7268, jiffies 4294943441 (age 20.470s)
hex dump (first 32 bytes):
00 40 7c 09 81 88 ff ff 80 45 b8 21 81 88 ff ff .@|......E.!....
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace:
[<0000000057619e23>] kmemleak_alloc_recursive include/linux/kmemleak.h:55 [inline]
[<0000000057619e23>] slab_post_alloc_hook mm/slab.h:439 [inline]
[<0000000057619e23>] slab_alloc mm/slab.c:3326 [inline]
[<0000000057619e23>] kmem_cache_alloc_trace+0x13d/0x280 mm/slab.c:3553
[<0000000086ce5479>] kmalloc include/linux/slab.h:547 [inline]
[<0000000086ce5479>] start_sync_thread+0x5d2/0xe10 net/netfilter/ipvs/ip_vs_sync.c:1862
[<000000001a9229cc>] do_ip_vs_set_ctl+0x4c5/0x780 net/netfilter/ipvs/ip_vs_ctl.c:2402
[<00000000ece457c8>] nf_sockopt net/netfilter/nf_sockopt.c:106 [inline]
[<00000000ece457c8>] nf_setsockopt+0x4c/0x80 net/netfilter/nf_sockopt.c:115
[<00000000942f62d4>] ip_setsockopt net/ipv4/ip_sockglue.c:1258 [inline]
[<00000000942f62d4>] ip_setsockopt+0x9b/0xb0 net/ipv4/ip_sockglue.c:1238
[<00000000a56a8ffd>] udp_setsockopt+0x4e/0x90 net/ipv4/udp.c:2616
[<00000000fa895401>] sock_common_setsockopt+0x38/0x50 net/core/sock.c:3130
[<0000000095eef4cf>] __sys_setsockopt+0x98/0x120 net/socket.c:2078
[<000000009747cf88>] __do_sys_setsockopt net/socket.c:2089 [inline]
[<000000009747cf88>] __se_sys_setsockopt net/socket.c:2086 [inline]
[<000000009747cf88>] __x64_sys_setsockopt+0x26/0x30 net/socket.c:2086
[<00000000ded8ba80>] do_syscall_64+0x76/0x1a0 arch/x86/entry/common.c:301
[<00000000893b4ac8>] entry_SYSCALL_64_after_hwframe+0x44/0xa9
Reported-by: syzbot+7e2e50c8adfccd2e5041@syzkaller.appspotmail.com
Suggested-by: Eric Biggers <ebiggers@kernel.org>
Fixes: 998e7a76804b ("ipvs: Use kthread_run() instead of doing a double-fork via kernel_thread()")
Signed-off-by: Julian Anastasov <ja@ssi.bg>
Acked-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
include/net/ip_vs.h | 6 +-
net/netfilter/ipvs/ip_vs_ctl.c | 4 --
net/netfilter/ipvs/ip_vs_sync.c | 134 +++++++++++++++++++++-------------------
3 files changed, 76 insertions(+), 68 deletions(-)
diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
index 2ac40135b576..b36a1df93e7c 100644
--- a/include/net/ip_vs.h
+++ b/include/net/ip_vs.h
@@ -808,11 +808,12 @@ struct ipvs_master_sync_state {
struct ip_vs_sync_buff *sync_buff;
unsigned long sync_queue_len;
unsigned int sync_queue_delay;
- struct task_struct *master_thread;
struct delayed_work master_wakeup_work;
struct netns_ipvs *ipvs;
};
+struct ip_vs_sync_thread_data;
+
/* How much time to keep dests in trash */
#define IP_VS_DEST_TRASH_PERIOD (120 * HZ)
@@ -943,7 +944,8 @@ struct netns_ipvs {
spinlock_t sync_lock;
struct ipvs_master_sync_state *ms;
spinlock_t sync_buff_lock;
- struct task_struct **backup_threads;
+ struct ip_vs_sync_thread_data *master_tinfo;
+ struct ip_vs_sync_thread_data *backup_tinfo;
int threads_mask;
volatile int sync_state;
struct mutex sync_mutex;
diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index 776c87ed4813..741d91aa4a8d 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -2396,9 +2396,7 @@ do_ip_vs_set_ctl(struct sock *sk, int cmd, void __user *user, unsigned int len)
cfg.syncid = dm->syncid;
ret = start_sync_thread(ipvs, &cfg, dm->state);
} else {
- mutex_lock(&ipvs->sync_mutex);
ret = stop_sync_thread(ipvs, dm->state);
- mutex_unlock(&ipvs->sync_mutex);
}
goto out_dec;
}
@@ -3515,10 +3513,8 @@ static int ip_vs_genl_del_daemon(struct netns_ipvs *ipvs, struct nlattr **attrs)
if (!attrs[IPVS_DAEMON_ATTR_STATE])
return -EINVAL;
- mutex_lock(&ipvs->sync_mutex);
ret = stop_sync_thread(ipvs,
nla_get_u32(attrs[IPVS_DAEMON_ATTR_STATE]));
- mutex_unlock(&ipvs->sync_mutex);
return ret;
}
diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c
index 2526be6b3d90..a4a78c4b06de 100644
--- a/net/netfilter/ipvs/ip_vs_sync.c
+++ b/net/netfilter/ipvs/ip_vs_sync.c
@@ -195,6 +195,7 @@ union ip_vs_sync_conn {
#define IPVS_OPT_F_PARAM (1 << (IPVS_OPT_PARAM-1))
struct ip_vs_sync_thread_data {
+ struct task_struct *task;
struct netns_ipvs *ipvs;
struct socket *sock;
char *buf;
@@ -374,8 +375,11 @@ static inline void sb_queue_tail(struct netns_ipvs *ipvs,
max(IPVS_SYNC_SEND_DELAY, 1));
ms->sync_queue_len++;
list_add_tail(&sb->list, &ms->sync_queue);
- if ((++ms->sync_queue_delay) == IPVS_SYNC_WAKEUP_RATE)
- wake_up_process(ms->master_thread);
+ if ((++ms->sync_queue_delay) == IPVS_SYNC_WAKEUP_RATE) {
+ int id = (int)(ms - ipvs->ms);
+
+ wake_up_process(ipvs->master_tinfo[id].task);
+ }
} else
ip_vs_sync_buff_release(sb);
spin_unlock(&ipvs->sync_lock);
@@ -1636,8 +1640,10 @@ static void master_wakeup_work_handler(struct work_struct *work)
spin_lock_bh(&ipvs->sync_lock);
if (ms->sync_queue_len &&
ms->sync_queue_delay < IPVS_SYNC_WAKEUP_RATE) {
+ int id = (int)(ms - ipvs->ms);
+
ms->sync_queue_delay = IPVS_SYNC_WAKEUP_RATE;
- wake_up_process(ms->master_thread);
+ wake_up_process(ipvs->master_tinfo[id].task);
}
spin_unlock_bh(&ipvs->sync_lock);
}
@@ -1703,10 +1709,6 @@ static int sync_thread_master(void *data)
if (sb)
ip_vs_sync_buff_release(sb);
- /* release the sending multicast socket */
- sock_release(tinfo->sock);
- kfree(tinfo);
-
return 0;
}
@@ -1740,11 +1742,6 @@ static int sync_thread_backup(void *data)
}
}
- /* release the sending multicast socket */
- sock_release(tinfo->sock);
- kfree(tinfo->buf);
- kfree(tinfo);
-
return 0;
}
@@ -1752,8 +1749,8 @@ static int sync_thread_backup(void *data)
int start_sync_thread(struct netns_ipvs *ipvs, struct ipvs_sync_daemon_cfg *c,
int state)
{
- struct ip_vs_sync_thread_data *tinfo = NULL;
- struct task_struct **array = NULL, *task;
+ struct ip_vs_sync_thread_data *ti = NULL, *tinfo;
+ struct task_struct *task;
struct net_device *dev;
char *name;
int (*threadfn)(void *data);
@@ -1822,7 +1819,7 @@ int start_sync_thread(struct netns_ipvs *ipvs, struct ipvs_sync_daemon_cfg *c,
threadfn = sync_thread_master;
} else if (state == IP_VS_STATE_BACKUP) {
result = -EEXIST;
- if (ipvs->backup_threads)
+ if (ipvs->backup_tinfo)
goto out_early;
ipvs->bcfg = *c;
@@ -1849,28 +1846,22 @@ int start_sync_thread(struct netns_ipvs *ipvs, struct ipvs_sync_daemon_cfg *c,
master_wakeup_work_handler);
ms->ipvs = ipvs;
}
- } else {
- array = kcalloc(count, sizeof(struct task_struct *),
- GFP_KERNEL);
- result = -ENOMEM;
- if (!array)
- goto out;
}
+ result = -ENOMEM;
+ ti = kcalloc(count, sizeof(struct ip_vs_sync_thread_data),
+ GFP_KERNEL);
+ if (!ti)
+ goto out;
for (id = 0; id < count; id++) {
- result = -ENOMEM;
- tinfo = kmalloc(sizeof(*tinfo), GFP_KERNEL);
- if (!tinfo)
- goto out;
+ tinfo = &ti[id];
tinfo->ipvs = ipvs;
- tinfo->sock = NULL;
if (state == IP_VS_STATE_BACKUP) {
+ result = -ENOMEM;
tinfo->buf = kmalloc(ipvs->bcfg.sync_maxlen,
GFP_KERNEL);
if (!tinfo->buf)
goto out;
- } else {
- tinfo->buf = NULL;
}
tinfo->id = id;
if (state == IP_VS_STATE_MASTER)
@@ -1885,17 +1876,15 @@ int start_sync_thread(struct netns_ipvs *ipvs, struct ipvs_sync_daemon_cfg *c,
result = PTR_ERR(task);
goto out;
}
- tinfo = NULL;
- if (state == IP_VS_STATE_MASTER)
- ipvs->ms[id].master_thread = task;
- else
- array[id] = task;
+ tinfo->task = task;
}
/* mark as active */
- if (state == IP_VS_STATE_BACKUP)
- ipvs->backup_threads = array;
+ if (state == IP_VS_STATE_MASTER)
+ ipvs->master_tinfo = ti;
+ else
+ ipvs->backup_tinfo = ti;
spin_lock_bh(&ipvs->sync_buff_lock);
ipvs->sync_state |= state;
spin_unlock_bh(&ipvs->sync_buff_lock);
@@ -1910,29 +1899,31 @@ int start_sync_thread(struct netns_ipvs *ipvs, struct ipvs_sync_daemon_cfg *c,
out:
/* We do not need RTNL lock anymore, release it here so that
- * sock_release below and in the kthreads can use rtnl_lock
- * to leave the mcast group.
+ * sock_release below can use rtnl_lock to leave the mcast group.
*/
rtnl_unlock();
- count = id;
- while (count-- > 0) {
- if (state == IP_VS_STATE_MASTER)
- kthread_stop(ipvs->ms[count].master_thread);
- else
- kthread_stop(array[count]);
+ id = min(id, count - 1);
+ if (ti) {
+ for (tinfo = ti + id; tinfo >= ti; tinfo--) {
+ if (tinfo->task)
+ kthread_stop(tinfo->task);
+ }
}
if (!(ipvs->sync_state & IP_VS_STATE_MASTER)) {
kfree(ipvs->ms);
ipvs->ms = NULL;
}
mutex_unlock(&ipvs->sync_mutex);
- if (tinfo) {
- if (tinfo->sock)
- sock_release(tinfo->sock);
- kfree(tinfo->buf);
- kfree(tinfo);
+
+ /* No more mutexes, release socks */
+ if (ti) {
+ for (tinfo = ti + id; tinfo >= ti; tinfo--) {
+ if (tinfo->sock)
+ sock_release(tinfo->sock);
+ kfree(tinfo->buf);
+ }
+ kfree(ti);
}
- kfree(array);
return result;
out_early:
@@ -1944,15 +1935,18 @@ int start_sync_thread(struct netns_ipvs *ipvs, struct ipvs_sync_daemon_cfg *c,
int stop_sync_thread(struct netns_ipvs *ipvs, int state)
{
- struct task_struct **array;
+ struct ip_vs_sync_thread_data *ti, *tinfo;
int id;
int retc = -EINVAL;
IP_VS_DBG(7, "%s(): pid %d\n", __func__, task_pid_nr(current));
+ mutex_lock(&ipvs->sync_mutex);
if (state == IP_VS_STATE_MASTER) {
+ retc = -ESRCH;
if (!ipvs->ms)
- return -ESRCH;
+ goto err;
+ ti = ipvs->master_tinfo;
/*
* The lock synchronizes with sb_queue_tail(), so that we don't
@@ -1971,38 +1965,56 @@ int stop_sync_thread(struct netns_ipvs *ipvs, int state)
struct ipvs_master_sync_state *ms = &ipvs->ms[id];
int ret;
+ tinfo = &ti[id];
pr_info("stopping master sync thread %d ...\n",
- task_pid_nr(ms->master_thread));
+ task_pid_nr(tinfo->task));
cancel_delayed_work_sync(&ms->master_wakeup_work);
- ret = kthread_stop(ms->master_thread);
+ ret = kthread_stop(tinfo->task);
if (retc >= 0)
retc = ret;
}
kfree(ipvs->ms);
ipvs->ms = NULL;
+ ipvs->master_tinfo = NULL;
} else if (state == IP_VS_STATE_BACKUP) {
- if (!ipvs->backup_threads)
- return -ESRCH;
+ retc = -ESRCH;
+ if (!ipvs->backup_tinfo)
+ goto err;
+ ti = ipvs->backup_tinfo;
ipvs->sync_state &= ~IP_VS_STATE_BACKUP;
- array = ipvs->backup_threads;
retc = 0;
for (id = ipvs->threads_mask; id >= 0; id--) {
int ret;
+ tinfo = &ti[id];
pr_info("stopping backup sync thread %d ...\n",
- task_pid_nr(array[id]));
- ret = kthread_stop(array[id]);
+ task_pid_nr(tinfo->task));
+ ret = kthread_stop(tinfo->task);
if (retc >= 0)
retc = ret;
}
- kfree(array);
- ipvs->backup_threads = NULL;
+ ipvs->backup_tinfo = NULL;
+ } else {
+ goto err;
}
+ id = ipvs->threads_mask;
+ mutex_unlock(&ipvs->sync_mutex);
+
+ /* No more mutexes, release socks */
+ for (tinfo = ti + id; tinfo >= ti; tinfo--) {
+ if (tinfo->sock)
+ sock_release(tinfo->sock);
+ kfree(tinfo->buf);
+ }
+ kfree(ti);
/* decrease the module use count */
ip_vs_use_count_dec();
+ return retc;
+err:
+ mutex_unlock(&ipvs->sync_mutex);
return retc;
}
@@ -2021,7 +2033,6 @@ void ip_vs_sync_net_cleanup(struct netns_ipvs *ipvs)
{
int retc;
- mutex_lock(&ipvs->sync_mutex);
retc = stop_sync_thread(ipvs, IP_VS_STATE_MASTER);
if (retc && retc != -ESRCH)
pr_err("Failed to stop Master Daemon\n");
@@ -2029,5 +2040,4 @@ void ip_vs_sync_net_cleanup(struct netns_ipvs *ipvs)
retc = stop_sync_thread(ipvs, IP_VS_STATE_BACKUP);
if (retc && retc != -ESRCH)
pr_err("Failed to stop Backup Daemon\n");
- mutex_unlock(&ipvs->sync_mutex);
}
--
2.11.0
^ permalink raw reply related
* Re: [PATCH v2 bpf-next] RV32G eBPF JIT
From: Luke Nelson @ 2019-06-28 17:41 UTC (permalink / raw)
To: Jiong Wang
Cc: LKML, Luke Nelson, Xi Wang, Palmer Dabbelt, Albert Ou,
Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
Yonghong Song, Björn Töpel, linux-riscv, Netdev, bpf
In-Reply-To: <87y31nuspw.fsf@netronome.com>
On Thu, Jun 27, 2019 at 5:18 AM Jiong Wang <jiong.wang@netronome.com> wrote:
>
> #define BPF_ZEXT_REG(DST)
> ((struct bpf_insn) {
> .code = BPF_ALU | BPF_MOV | BPF_X
>
> So it can't be BPF_ALU64. It is safe to remove this chunk of code.
>
Thanks! I'll fix this in the next revision.
- Luke
^ permalink raw reply
* Re: [PATCH 1/3, net-next] net: netsec: Use page_pool API
From: Jesper Dangaard Brouer @ 2019-06-28 17:41 UTC (permalink / raw)
To: Ilias Apalodimas
Cc: David Miller, netdev, jaswinder.singh, ard.biesheuvel,
bjorn.topel, magnus.karlsson, daniel, ast, makita.toshiaki,
jakub.kicinski, john.fastabend, maciejromanfijalkowski, brouer
In-Reply-To: <20190628171934.GA31070@apalos>
On Fri, 28 Jun 2019 20:19:34 +0300
Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote:
> Hi David,
>
> > >> Use page_pool and it's DMA mapping capabilities for Rx buffers instead
> > >> of netdev/napi_alloc_frag()
> > >>
> > >> Although this will result in a slight performance penalty on small sized
> > >> packets (~10%) the use of the API will allow to easily add XDP support.
> > >> The penalty won't be visible in network testing i.e ipef/netperf etc, it
> > >> only happens during raw packet drops.
> > >> Furthermore we intend to add recycling capabilities on the API
> > >> in the future. Once the recycling is added the performance penalty will
> > >> go away.
> > >> The only 'real' penalty is the slightly increased memory usage, since we
> > >> now allocate a page per packet instead of the amount of bytes we need +
> > >> skb metadata (difference is roughly 2kb per packet).
> > >> With a minimum of 4BG of RAM on the only SoC that has this NIC the
> > >> extra memory usage is negligible (a bit more on 64K pages)
> > >>
> > >> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > >> ---
> > >> drivers/net/ethernet/socionext/Kconfig | 1 +
> > >> drivers/net/ethernet/socionext/netsec.c | 121 +++++++++++++++---------
> > >> 2 files changed, 75 insertions(+), 47 deletions(-)
> > >
> > > Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
> >
> > Jesper this is confusing, you just asked if the code needs to be moved
> > around to be correct and then right now immediately afterwards you ACK
> > the patch.
>
> I can answer on the driver, page_pool_free() needs re-arranging
> indeed. I'll fix it and post a V2. I guess Jesper meant
> 'acked-if-fixed' so i can it on V2
Sorry, it was a mistake. I though I had spotted an issue in 3/3 and
then I wanted to ACK 1/3. Ilias you can add my ACK in V2, as this was
the only issue I spotted in 1/3.
Sorry for the confusion.
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: [PATCH bpf-next v3 1/2] xsk: remove AF_XDP socket from map when the socket is released
From: Björn Töpel @ 2019-06-28 17:33 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Alexei Starovoitov, Netdev, Björn Töpel,
Karlsson, Magnus, Bruce Richardson, Song Liu, bpf
In-Reply-To: <2417e1ab-16fa-d3ed-564e-1a50c4cb6717@iogearbox.net>
On Fri, 28 Jun 2019 at 02:33, Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 06/20/2019 12:06 PM, Björn Töpel wrote:
> > From: Björn Töpel <bjorn.topel@intel.com>
> >
> > When an AF_XDP socket is released/closed the XSKMAP still holds a
> > reference to the socket in a "released" state. The socket will still
> > use the netdev queue resource, and block newly created sockets from
> > attaching to that queue, but no user application can access the
> > fill/complete/rx/tx queues. This results in that all applications need
> > to explicitly clear the map entry from the old "zombie state"
> > socket. This should be done automatically.
> >
> > After this patch, when a socket is released, it will remove itself
> > from all the XSKMAPs it resides in, allowing the socket application to
> > remove the code that cleans the XSKMAP entry.
> >
> > This behavior is also closer to that of SOCKMAP, making the two socket
> > maps more consistent.
> >
> > Suggested-by: Bruce Richardson <bruce.richardson@intel.com>
> > Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
>
> Sorry for the bit of delay in reviewing, few comments inline:
>
No worries!
> > ---
> > include/net/xdp_sock.h | 3 ++
> > kernel/bpf/xskmap.c | 101 +++++++++++++++++++++++++++++++++++------
> > net/xdp/xsk.c | 25 ++++++++++
> > 3 files changed, 116 insertions(+), 13 deletions(-)
> >
> > diff --git a/include/net/xdp_sock.h b/include/net/xdp_sock.h
> > index ae0f368a62bb..011a1b08d7c9 100644
> > --- a/include/net/xdp_sock.h
> > +++ b/include/net/xdp_sock.h
> > @@ -68,6 +68,8 @@ struct xdp_sock {
> > */
> > spinlock_t tx_completion_lock;
> > u64 rx_dropped;
> > + struct list_head map_list;
> > + spinlock_t map_list_lock;
> > };
> >
> > struct xdp_buff;
> > @@ -87,6 +89,7 @@ struct xdp_umem_fq_reuse *xsk_reuseq_swap(struct xdp_umem *umem,
> > struct xdp_umem_fq_reuse *newq);
> > void xsk_reuseq_free(struct xdp_umem_fq_reuse *rq);
> > struct xdp_umem *xdp_get_umem_from_qid(struct net_device *dev, u16 queue_id);
> > +void xsk_map_delete_from_node(struct xdp_sock *xs, struct list_head *node);
> >
> > static inline char *xdp_umem_get_data(struct xdp_umem *umem, u64 addr)
> > {
> > diff --git a/kernel/bpf/xskmap.c b/kernel/bpf/xskmap.c
> > index ef7338cebd18..af802c89ebab 100644
> > --- a/kernel/bpf/xskmap.c
> > +++ b/kernel/bpf/xskmap.c
> > @@ -13,8 +13,58 @@ struct xsk_map {
> > struct bpf_map map;
> > struct xdp_sock **xsk_map;
> > struct list_head __percpu *flush_list;
> > + spinlock_t lock;
> > };
> >
> > +/* Nodes are linked in the struct xdp_sock map_list field, and used to
> > + * track which maps a certain socket reside in.
> > + */
> > +struct xsk_map_node {
> > + struct list_head node;
> > + struct xsk_map *map;
> > + struct xdp_sock **map_entry;
> > +};
> > +
> > +static struct xsk_map_node *xsk_map_node_alloc(void)
> > +{
> > + return kzalloc(sizeof(struct xsk_map_node), GFP_ATOMIC | __GFP_NOWARN);
> > +}
> > +
> > +static void xsk_map_node_free(struct xsk_map_node *node)
> > +{
> > + kfree(node);
> > +}
> > +
> > +static void xsk_map_node_init(struct xsk_map_node *node,
> > + struct xsk_map *map,
> > + struct xdp_sock **map_entry)
> > +{
> > + node->map = map;
> > + node->map_entry = map_entry;
> > +}
> > +
> > +static void xsk_map_add_node(struct xdp_sock *xs, struct xsk_map_node *node)
> > +{
> > + spin_lock_bh(&xs->map_list_lock);
> > + list_add_tail(&node->node, &xs->map_list);
> > + spin_unlock_bh(&xs->map_list_lock);
> > +}
> > +
> > +static void xsk_map_del_node(struct xdp_sock *xs, struct xdp_sock **map_entry)
> > +{
> > + struct xsk_map_node *n, *tmp;
> > +
> > + spin_lock_bh(&xs->map_list_lock);
> > + list_for_each_entry_safe(n, tmp, &xs->map_list, node) {
> > + if (map_entry == n->map_entry) {
> > + list_del(&n->node);
> > + xsk_map_node_free(n);
> > + }
> > + }
> > + spin_unlock_bh(&xs->map_list_lock);
> > +
> > +}
> > +
> > static struct bpf_map *xsk_map_alloc(union bpf_attr *attr)
> > {
> > struct xsk_map *m;
> > @@ -34,6 +84,7 @@ static struct bpf_map *xsk_map_alloc(union bpf_attr *attr)
> > return ERR_PTR(-ENOMEM);
> >
> > bpf_map_init_from_attr(&m->map, attr);
> > + spin_lock_init(&m->lock);
> >
> > cost = (u64)m->map.max_entries * sizeof(struct xdp_sock *);
> > cost += sizeof(struct list_head) * num_possible_cpus();
> > @@ -76,15 +127,16 @@ static void xsk_map_free(struct bpf_map *map)
> > bpf_clear_redirect_map(map);
> > synchronize_net();
> >
> > + spin_lock_bh(&m->lock);
> > for (i = 0; i < map->max_entries; i++) {
> > - struct xdp_sock *xs;
> > -
> > - xs = m->xsk_map[i];
> > - if (!xs)
> > - continue;
> > + struct xdp_sock **map_entry = &m->xsk_map[i];
> > + struct xdp_sock *old_xs;
> >
> > - sock_put((struct sock *)xs);
> > + old_xs = xchg(map_entry, NULL);
> > + if (old_xs)
> > + xsk_map_del_node(old_xs, map_entry);
> > }
> > + spin_unlock_bh(&m->lock);
> >
> > free_percpu(m->flush_list);
> > bpf_map_area_free(m->xsk_map);
> > @@ -166,7 +218,8 @@ static int xsk_map_update_elem(struct bpf_map *map, void *key, void *value,
> > {
> > struct xsk_map *m = container_of(map, struct xsk_map, map);
> > u32 i = *(u32 *)key, fd = *(u32 *)value;
> > - struct xdp_sock *xs, *old_xs;
> > + struct xdp_sock *xs, *old_xs, **entry;
> > + struct xsk_map_node *node;
> > struct socket *sock;
> > int err;
> >
> > @@ -193,11 +246,20 @@ static int xsk_map_update_elem(struct bpf_map *map, void *key, void *value,
> > return -EOPNOTSUPP;
> > }
> >
> > - sock_hold(sock->sk);
> > + node = xsk_map_node_alloc();
> > + if (!node) {
> > + sockfd_put(sock);
> > + return -ENOMEM;
> > + }
> >
> > - old_xs = xchg(&m->xsk_map[i], xs);
> > + spin_lock_bh(&m->lock);
> > + entry = &m->xsk_map[i];
> > + xsk_map_node_init(node, m, entry);
> > + xsk_map_add_node(xs, node);
> > + old_xs = xchg(entry, xs);
> > if (old_xs)
> > - sock_put((struct sock *)old_xs);
> > + xsk_map_del_node(old_xs, entry);
> > + spin_unlock_bh(&m->lock);
> >
> > sockfd_put(sock);
> > return 0;
> > @@ -206,19 +268,32 @@ static int xsk_map_update_elem(struct bpf_map *map, void *key, void *value,
> > static int xsk_map_delete_elem(struct bpf_map *map, void *key)
> > {
> > struct xsk_map *m = container_of(map, struct xsk_map, map);
> > - struct xdp_sock *old_xs;
> > + struct xdp_sock *old_xs, **map_entry;
> > int k = *(u32 *)key;
> >
> > if (k >= map->max_entries)
> > return -EINVAL;
> >
> > - old_xs = xchg(&m->xsk_map[k], NULL);
> > + spin_lock_bh(&m->lock);
> > + map_entry = &m->xsk_map[k];
> > + old_xs = xchg(map_entry, NULL);
> > if (old_xs)
> > - sock_put((struct sock *)old_xs);
> > + xsk_map_del_node(old_xs, map_entry);
> > + spin_unlock_bh(&m->lock);
> >
> > return 0;
> > }
> >
> > +void xsk_map_delete_from_node(struct xdp_sock *xs, struct list_head *node)
> > +{
> > + struct xsk_map_node *n = list_entry(node, struct xsk_map_node, node);
> > +
> > + spin_lock_bh(&n->map->lock);
> > + *n->map_entry = NULL;
> > + spin_unlock_bh(&n->map->lock);
> > + xsk_map_node_free(n);
> > +}
> > +
> > const struct bpf_map_ops xsk_map_ops = {
> > .map_alloc = xsk_map_alloc,
> > .map_free = xsk_map_free,
> > diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> > index a14e8864e4fa..1931d98a7754 100644
> > --- a/net/xdp/xsk.c
> > +++ b/net/xdp/xsk.c
> > @@ -335,6 +335,27 @@ static int xsk_init_queue(u32 entries, struct xsk_queue **queue,
> > return 0;
> > }
> >
> > +static struct list_head *xsk_map_list_pop(struct xdp_sock *xs)
> > +{
> > + struct list_head *node = NULL;
> > +
> > + spin_lock_bh(&xs->map_list_lock);
> > + if (!list_empty(&xs->map_list)) {
> > + node = xs->map_list.next;
> > + list_del(node);
> > + }
> > + spin_unlock_bh(&xs->map_list_lock);
> > + return node;
> > +}
> > +
> > +static void xsk_delete_from_maps(struct xdp_sock *xs)
> > +{
> > + struct list_head *node;
> > +
> > + while ((node = xsk_map_list_pop(xs)))
> > + xsk_map_delete_from_node(xs, node);
> > +}
> > +
>
> I stared at this set for a while and I think there are still two
> issues in the design unless I'm missing something obvious.
>
> 1) xs teardown and parallel map update:
>
> - CPU0 is in xsk_release(), calls xsk_delete_from_maps().
> - CPU1 is in xsk_map_update_elem(), both access the same map slot.
> - CPU0 does the xsk_map_list_pop() for that given slot, gets
> interrupted before calling into xsk_map_delete_from_node().
> - CPU1 takes m->lock in updates, *entry = xs to the new sock,
> does xsk_map_del_node() to check on the xs (which CPU0 tears
> down). Given this was popped off the list, it doesn't do
> anything here, all good. It unlocks m->lock and succeeds.
> - CPU0 now continues in xsk_map_delete_from_node(), takes
> m->lock, zeroes *n->map_entry, releases m->lock, and frees
> n. However, at this point *n->map_entry contains the xs that
> we've just updated on CPU1. So zero'ing it will 1) remove
> the wrong entry, and ii) leak it since it goes out of reach.
>
Argh. No, you're (again) not missing anything. Thanks for catching this.
> 2) Inconsistent use of xchg() and friends:
>
> - AF_XDP fast-path is doing READ_ONCE(m->xsk_map[key]) without
> taking m->lock. This is also why you have xchg() for example
> inside m->lock region since both protect different things (should
> probably be commented). However, this is not consistently used.
> E.g. xsk_map_delete_from_node() or xsk_map_update_elem() have
> plain assignment, so compiler could in theory happily perform
> store tearing and the READ_ONCE() would see garbage. This needs
> to be consistently paired.
>
Indeed. Back to the drawing board. Thanks for your time, Daniel.
Björn
> > static int xsk_release(struct socket *sock)
> > {
> > struct sock *sk = sock->sk;
> > @@ -354,6 +375,7 @@ static int xsk_release(struct socket *sock)
> > sock_prot_inuse_add(net, sk->sk_prot, -1);
> > local_bh_enable();
> >
> > + xsk_delete_from_maps(xs);
> > if (xs->dev) {
> > struct net_device *dev = xs->dev;
> >
> > @@ -767,6 +789,9 @@ static int xsk_create(struct net *net, struct socket *sock, int protocol,
> > mutex_init(&xs->mutex);
> > spin_lock_init(&xs->tx_completion_lock);
> >
> > + INIT_LIST_HEAD(&xs->map_list);
> > + spin_lock_init(&xs->map_list_lock);
> > +
> > mutex_lock(&net->xdp.lock);
> > sk_add_node_rcu(sk, &net->xdp.list);
> > mutex_unlock(&net->xdp.lock);
> >
>
^ permalink raw reply
* Re: [PATCH] netfilter: Fix remainder of pseudo-header protocol 0
From: Pablo Neira Ayuso @ 2019-06-28 17:30 UTC (permalink / raw)
To: zhe.he; +Cc: kadlec, fw, davem, netfilter-devel, coreteam, netdev,
linux-kernel
In-Reply-To: <1561346258-272481-1-git-send-email-zhe.he@windriver.com>
On Mon, Jun 24, 2019 at 11:17:38AM +0800, zhe.he@windriver.com wrote:
> From: He Zhe <zhe.he@windriver.com>
>
> Since v5.1-rc1, some types of packets do not get unreachable reply with the
> following iptables setting. Fox example,
>
> $ iptables -A INPUT -p icmp --icmp-type 8 -j REJECT
> $ ping 127.0.0.1 -c 1
> PING 127.0.0.1 (127.0.0.1) 56(84) bytes of data.
> — 127.0.0.1 ping statistics —
> 1 packets transmitted, 0 received, 100% packet loss, time 0ms
>
> We should have got the following reply from command line, but we did not.
> From 127.0.0.1 icmp_seq=1 Destination Port Unreachable
>
> Yi Zhao reported it and narrowed it down to:
> 7fc38225363d ("netfilter: reject: skip csum verification for protocols that don't support it"),
>
> This is because nf_ip_checksum still expects pseudo-header protocol type 0 for
> packets that are of neither TCP or UDP, and thus ICMP packets are mistakenly
> treated as TCP/UDP.
>
> This patch corrects the conditions in nf_ip_checksum and all other places that
> still call it with protocol 0.
Applied, thanks.
^ permalink raw reply
* Re: [PATCH 3/3, net-next] net: netsec: add XDP support
From: Maciej Fijalkowski @ 2019-06-28 17:23 UTC (permalink / raw)
To: Ilias Apalodimas
Cc: Jesper Dangaard Brouer, netdev, jaswinder.singh, ard.biesheuvel,
bjorn.topel, magnus.karlsson, daniel, ast, makita.toshiaki,
jakub.kicinski, john.fastabend, davem
In-Reply-To: <20190628164741.GA27936@apalos>
On Fri, 28 Jun 2019 19:47:41 +0300
Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote:
> On Fri, Jun 28, 2019 at 03:35:52PM +0200, Jesper Dangaard Brouer wrote:
> > On Fri, 28 Jun 2019 13:39:15 +0300
> > Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote:
> >
> > > +static int netsec_xdp_setup(struct netsec_priv *priv, struct bpf_prog *prog,
> > > + struct netlink_ext_ack *extack)
> > > +{
> > > + struct net_device *dev = priv->ndev;
> > > + struct bpf_prog *old_prog;
> > > +
> > > + /* For now just support only the usual MTU sized frames */
> > > + if (prog && dev->mtu > 1500) {
> > > + NL_SET_ERR_MSG_MOD(extack, "Jumbo frames not supported on XDP");
> > > + return -EOPNOTSUPP;
> > > + }
> > > +
> > > + if (netif_running(dev))
> > > + netsec_netdev_stop(dev);
> > > +
> > > + /* Detach old prog, if any */
> > > + old_prog = xchg(&priv->xdp_prog, prog);
> > > + if (old_prog)
> > > + bpf_prog_put(old_prog);
> > > +
> > > + if (netif_running(dev))
> > > + netsec_netdev_open(dev);
> >
> > Shouldn't the if-statement be if (!netif_running(dev))
> >
> > > +
> This is there to restart the device if it's up already (to rebuild the rings).
> This should be fine as-is
I think that Jesper's concern was about that you could have already stopped the
netdev earlier via netsec_netdev_stop (before the xchg)? So at this point
__LINK_STATE_START might be not set.
Maybe initially store what netif_running(dev) returns in stack variable and
act on it, so your stop/open are symmetric?
>
> > > + return 0;
> > > +}
>
> Thanks
> /Ilias
^ permalink raw reply
* Re: [PATCH 1/1] net: dsa: b53: Disable all ports on setup
From: Florian Fainelli @ 2019-06-28 17:23 UTC (permalink / raw)
To: Benedikt Spranger
Cc: netdev, Sebastian Andrzej Siewior, Kurt Kanzenbach, Andrew Lunn,
Vivien Didelot
In-Reply-To: <20190628165811.30964-2-b.spranger@linutronix.de>
On 6/28/19 9:58 AM, Benedikt Spranger wrote:
> A b53 device may configured through an external EEPROM like the switch
> device on the Lamobo R1 router board. The configuration of a port may
> therefore differ from the reset configuration of the switch.
>
> The switch configuration reported by the DSA subsystem is different until
> the port is configured by DSA i.e. a port can be active, while the DSA
> subsystem reports the port is inactive. Disable all ports and not only
> the unused ones to put all ports into a well defined state.
>
> Signed-off-by: Benedikt Spranger <b.spranger@linutronix.de>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Makes sense, in fact, that should probably be moved to the DSA core at
some point (wink wink Vivien).
--
Florian
^ permalink raw reply
* Re: [PATCH 1/3, net-next] net: netsec: Use page_pool API
From: Ilias Apalodimas @ 2019-06-28 17:19 UTC (permalink / raw)
To: David Miller
Cc: brouer, netdev, jaswinder.singh, ard.biesheuvel, bjorn.topel,
magnus.karlsson, daniel, ast, makita.toshiaki, jakub.kicinski,
john.fastabend, maciejromanfijalkowski
In-Reply-To: <20190628.094343.1065314747200152509.davem@davemloft.net>
Hi David,
> >> Use page_pool and it's DMA mapping capabilities for Rx buffers instead
> >> of netdev/napi_alloc_frag()
> >>
> >> Although this will result in a slight performance penalty on small sized
> >> packets (~10%) the use of the API will allow to easily add XDP support.
> >> The penalty won't be visible in network testing i.e ipef/netperf etc, it
> >> only happens during raw packet drops.
> >> Furthermore we intend to add recycling capabilities on the API
> >> in the future. Once the recycling is added the performance penalty will
> >> go away.
> >> The only 'real' penalty is the slightly increased memory usage, since we
> >> now allocate a page per packet instead of the amount of bytes we need +
> >> skb metadata (difference is roughly 2kb per packet).
> >> With a minimum of 4BG of RAM on the only SoC that has this NIC the
> >> extra memory usage is negligible (a bit more on 64K pages)
> >>
> >> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> >> ---
> >> drivers/net/ethernet/socionext/Kconfig | 1 +
> >> drivers/net/ethernet/socionext/netsec.c | 121 +++++++++++++++---------
> >> 2 files changed, 75 insertions(+), 47 deletions(-)
> >
> > Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
>
> Jesper this is confusing, you just asked if the code needs to be moved
> around to be correct and then right now immediately afterwards you ACK
> the patch.
I can answer on the driver, page_pool_free() needs re-arranging indeed.
I'll fix it and post a V2. I guess Jesper meant 'acked-if-fixed' so i can it on
V2
Thanks
/Ilias
^ permalink raw reply
* [PATCH 1/1] net: dsa: b53: Disable all ports on setup
From: Benedikt Spranger @ 2019-06-28 16:58 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev, Sebastian Andrzej Siewior, Kurt Kanzenbach, Andrew Lunn,
Vivien Didelot
In-Reply-To: <20190628165811.30964-1-b.spranger@linutronix.de>
A b53 device may configured through an external EEPROM like the switch
device on the Lamobo R1 router board. The configuration of a port may
therefore differ from the reset configuration of the switch.
The switch configuration reported by the DSA subsystem is different until
the port is configured by DSA i.e. a port can be active, while the DSA
subsystem reports the port is inactive. Disable all ports and not only
the unused ones to put all ports into a well defined state.
Signed-off-by: Benedikt Spranger <b.spranger@linutronix.de>
---
drivers/net/dsa/b53/b53_common.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index a47f5bc667bd..5127c2fefba9 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -962,13 +962,13 @@ static int b53_setup(struct dsa_switch *ds)
if (ret)
dev_err(ds->dev, "failed to apply configuration\n");
- /* Configure IMP/CPU port, disable unused ports. Enabled
+ /* Configure IMP/CPU port, disable all other ports. Enabled
* ports will be configured with .port_enable
*/
for (port = 0; port < dev->num_ports; port++) {
if (dsa_is_cpu_port(ds, port))
b53_enable_cpu_port(dev, port);
- else if (dsa_is_unused_port(ds, port))
+ else
b53_disable_port(ds, port);
}
--
2.20.1
^ permalink raw reply related
* [PATCH 0/1] Disable all ports on b53 setup
From: Benedikt Spranger @ 2019-06-28 16:58 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev, Sebastian Andrzej Siewior, Kurt Kanzenbach, Andrew Lunn,
Vivien Didelot
Hi,
while working on a Banana Pi R1 based system I faced inconsistent
switch configurations. The switch is attached to an EEPROM which feeds
additional configuration which is applied after the reset of the chip.
As a result all ports remained active while the DSA subsystem
assumed that those ports were inactive after the reset.
Disable the ports on switch setup to get a consistent view of things
between real life and DSA.
Benedikt Spranger (1):
net: dsa: b53: Disable all ports on setup
drivers/net/dsa/b53/b53_common.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--
2.20.1
^ permalink raw reply
* Re: [RFC iproute2 1/1] ip: netns: add mounted state file for each netns
From: Matteo Croce @ 2019-06-28 17:06 UTC (permalink / raw)
To: David Howells
Cc: Nicolas Dichtel, Alexander Aring, netdev, linux-fsdevel, kernel
In-Reply-To: <18557.1561739215@warthog.procyon.org.uk>
On Fri, Jun 28, 2019 at 6:27 PM David Howells <dhowells@redhat.com> wrote:
>
> Nicolas Dichtel <nicolas.dichtel@6wind.com> wrote:
>
> > David Howells was working on a mount notification mechanism:
> > https://lwn.net/Articles/760714/
> > https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/log/?h=notifications
> >
> > I don't know what is the status of this series.
>
> It's still alive. I just posted a new version on it. I'm hoping, possibly
> futiley, to get it in in this merge window.
>
> David
Hi all,
this could cause a clash if I create a netns with name ending with .mounted.
$ sudo ip/ip netns add ns1.mounted
$ sudo ip/ip netns add ns1
Cannot create namespace file "/var/run/netns/ns1.mounted": File exists
Cannot remove namespace file "/var/run/netns/ns1.mounted": Device or
resource busy
If you want to go along this road, please either:
- disallow netns creation with names ending with .mounted
- or properly document it in the manpage
Regards,
--
Matteo Croce
per aspera ad upstream
^ permalink raw reply
* Re: [RFC PATCH 1/1] Documentation: net: dsa: b53: Describe b53 configuration
From: Benedikt Spranger @ 2019-06-28 16:57 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev, Sebastian Andrzej Siewior, Kurt Kanzenbach, Andrew Lunn,
Vivien Didelot
In-Reply-To: <5fe6c1b8-6273-be3d-cf75-6efdd7f9b27d@gmail.com>
Am Thu, 27 Jun 2019 09:38:16 -0700
schrieb Florian Fainelli <f.fainelli@gmail.com>:
> On 6/27/19 3:15 AM, Benedikt Spranger wrote:
> > Document the different needs of documentation for the b53 driver.
> >
> > Signed-off-by: Benedikt Spranger <b.spranger@linutronix.de>
> > ---
> > Documentation/networking/dsa/b53.rst | 300
> > +++++++++++++++++++++++++++ 1 file changed, 300 insertions(+)
> > create mode 100644 Documentation/networking/dsa/b53.rst
> >
> > diff --git a/Documentation/networking/dsa/b53.rst
> > b/Documentation/networking/dsa/b53.rst new file mode 100644
> > index 000000000000..5838cf6230da
> > --- /dev/null
> > +++ b/Documentation/networking/dsa/b53.rst
> > @@ -0,0 +1,300 @@
> > +.. SPDX-License-Identifier: GPL-2.0
> > +
> > +==========================================
> > +Broadcom RoboSwitch Ethernet switch driver
> > +==========================================
> > +
> > +The Broadcom RoboSwitch Ethernet switch family is used in quite a
> > range of +xDSL router, cable modems and other multimedia devices.
> > +
> > +The actual implementation supports the devices BCM5325E, BCM5365,
> > BCM539x, +BCM53115 and BCM53125 as well as BCM63XX.
> > +
> > +Implementation details
> > +======================
> > +
> > +The driver is located in ``drivers/net/dsa/bcm_sf2.c`` and is
> > implemented as a +DSA driver; see
> > ``Documentation/networking/dsa/dsa.rst`` for details on the
> > +subsystemand what it provides.
>
> The driver is under drivers/net/dsa/b53/
Fixed.
> s/ethernet/Ethernet/ for your global submission.
OK.
> What you are describing is not entirely specific to the B53 driver
> (maybe with the exception of having a VLAN tag on the DSA master
> network device, since B53 puts the CPU port tagged in all VLANs by
> default), and therefore the entire document should be written with
> the general DSA devices in mind, and eventually pointing out where
> B53 differs in a separate document.
I have split up the Documentation into
Documentation/networking/dsa/configuration.rst
and
Documentation/networking/dsa/b53.rst
> There are largely two kinds of behavior:
>
> - switches that are configured with DSA_TAG_PROTO_NONE, which behave
> in a certain way and require a bridge with VLAN filtering even when
> ports are intended to be used as standalone devices.
>
> - switches that are configured with a tagging protocol other than
> DSA_TAG_PROTO_NONE, which behave more like traditional network devices
> people are used to use.
OK.
> > +bridge
> > +~~~~~~
>
> I would add something like:
>
> All ports being part of a single bridge/broadcast domain or something
> along those lines. Seeing the "wan" interface being added to a bridge
> is a bit confusing.
> > +
> > +.. code-block:: sh
> > +
> > + # create bridge
> > + ip link add name br0 type bridge
> > +
> > + # add ports to bridge
> > + ip link set dev wan master br0
> > + ip link set dev lan1 master br0
> > + ip link set dev lan2 master br0
> > +
> > + # configure the bridge
> > + ip addr add 192.0.2.129/25 dev br0
> > +
> > + # The master interface needs to be brought up before the slave
> > ports.
> > + ip link set eth0 up
> > +
> > + # bring up the slave interfaces
> > + ip link set wan up
> > + ip link set lan1 up
> > + ip link set lan2 up
> > +
> > + # bring up the bridge
> > + ip link set dev br0 up
> > +
> > +gateway
> > +~~~~~~~
> > +
> > +.. code-block:: sh
> > +
> > + # create bridge
> > + ip link add name br0 type bridge
> > +
> > + # add ports to bridge
> > + ip link set dev lan1 master br0
> > + ip link set dev lan2 master br0
> > +
> > + # configure the bridge
> > + ip addr add 192.0.2.129/25 dev br0
> > +
> > + # configure the upstream port
> > + ip addr add 192.0.2.1/30 dev wan
> > +
> > + # The master interface needs to be brought up before the slave
> > ports.
> > + ip link set eth0 up
> > +
> > + # bring up the slave interfaces
> > + ip link set wan up
> > + ip link set lan1 up
> > + ip link set lan2 up
> > +
> > + # bring up the bridge
> > + ip link set dev br0 up
> > +
> > +Configuration without tagging support
> > +-------------------------------------
> > +
> > +Older models (5325, 5365) support a different tag format that is
> > not supported +yet. 539x and 531x5 require managed mode and some
> > special handling, which is +also not yet supported. The tagging
> > support is disabled in these cases and the +switch need a different
> > configuration.
>
> On that topic, did the patch I sent you ended up working the way you
> wanted it with ifupdown-scripts or are you still chasing some other
> issues with it?
Your patch is needed and working. Stumbled over my own feed.
I have a hackary solution by now. Hackary in terms of not tracking the
master interface state (just *up* it unconditionally).
> > +
> > +single port
> > +~~~~~~~~~~~
> > +The configuration can only be set up via VLAN tagging and bridge
> > setup. +By default packages are tagged with vid 1:
> > +
> > +.. code-block:: sh
> > +
> > + # tag traffic on CPU port
> > + ip link add link eth0 name eth0.1 type vlan id 1
> > + ip link add link eth0 name eth0.2 type vlan id 2
> > + ip link add link eth0 name eth0.3 type vlan id 3
>
> That part is indeed a B53 implementation specific detail because B53
> tags the CPU port in all VLANs, since otherwise any PVID untagged VLAN
> programming would basically change the CPU port's default PVID and
> make it untagged, undesirable.
OK.
> > +
> > + # create bridges
> > + ip link add name br0 type bridge
> > + ip link add name br1 type bridge
> > + ip link add name br2 type bridge
> > +
> > + # activate VLAN filtering
> > + ip link set dev br0 type bridge vlan_filtering 1
> > + ip link set dev br1 type bridge vlan_filtering 1
> > + ip link set dev br2 type bridge vlan_filtering 1
> > +
> > + # add ports to bridges
> > + ip link set dev wan master br0
> > + ip link set eth0.1 master br0
> > + ip link set dev lan1 master br1
> > + ip link set eth0.2 master br1
> > + ip link set dev lan2 master br2
> > + ip link set eth0.3 master br2
>
> I don't think you need one bridge for each port you want to isolate
> with DSA_PROTO_TAG_NONE, you can just have a single bridge and assign
> the ports a different VLAN with the commands below:
Tried that out:
# tag traffic on CPU port
ip link add link eth0 name eth0.1 type vlan id 1
ip link add link eth0 name eth0.2 type vlan id 2
ip link add link eth0 name eth0.3 type vlan id 3
# The master interface needs to be brought up before the slave ports.
ip link set eth0 up
ip link set eth0.1 up
ip link set eth0.2 up
ip link set eth0.3 up
# bring up the slave interfaces
ip link set wan up
ip link set lan1 up
ip link set lan2 up
# create bridge
ip link add name br0 type bridge
# activate VLAN filtering
ip link set dev br0 type bridge vlan_filtering 1
# add ports to bridges
ip link set dev wan master br0
ip link set dev lan1 master br0
ip link set dev lan2 master br0
# tag traffic on ports
bridge vlan add dev lan1 vid 2 pvid untagged
bridge vlan del dev lan1 vid 1
bridge vlan add dev lan2 vid 3 pvid untagged
bridge vlan del dev lan2 vid 1
# configure the VLANs
ip addr add 192.0.2.1/30 dev eth0.1
ip addr add 192.0.2.5/30 dev eth0.2
ip addr add 192.0.2.9/30 dev eth0.3
# bring up the bridge devices
ip link set br0 up
Works quite well :)
> > +
> > + # tag traffic on ports
> > + bridge vlan add dev lan1 vid 2 pvid untagged
> > + bridge vlan del dev lan1 vid 1
> > + bridge vlan add dev lan2 vid 3 pvid untagged
> > + bridge vlan del dev lan2 vid 1
>
> And also permit the different VLANs that you created on the bridge
> master device itself:
>
> bridge vlan add vid 2 dev br0 self
> bridve vlan add vid 3 dev br0 self
>
> Maybe that last part above ^^ was missing and that's why people tend
> to create multiple bridge devices?
From my side there was some tree in the woods problem...
> > +
> > + # configure the bridges
> > + ip addr add 192.0.2.1/30 dev br0
> > + ip addr add 192.0.2.5/30 dev br1
> > + ip addr add 192.0.2.9/30 dev br2
> > +
> > + # The master interface needs to be brought up before the slave
> > ports.
> > + ip link set eth0 up
> > + ip link set eth0.1 up
> > + ip link set eth0.2 up
> > + ip link set eth0.3 up
> > +
> > + # bring up the slave interfaces
> > + ip link set wan up
> > + ip link set lan1 up
> > + ip link set lan2 up
> > +
> > + # bring up the bridge devices
> > + ip link set br0 up
> > + ip link set br1 up
> > + ip link set br2 up
> > +
> > +bridge
> > +~~~~~~
> > +
> > +.. code-block:: sh
> > +
> > + # tag traffic on CPU port
> > + ip link add link eth0 name eth0.1 type vlan id 1
> > +
> > + # create bridge
> > + ip link add name br0 type bridge
> > +
> > + # activate VLAN filtering
> > + ip link set dev br0 type bridge vlan_filtering 1
> > +
> > + # add ports to bridge
> > + ip link set dev wan master br0
> > + ip link set dev lan1 master br0
> > + ip link set dev lan2 master br0
> > + ip link set eth0.1 master br0
> > +
> > + # configure the bridge
> > + ip addr add 192.0.2.129/25 dev br0
> > +
> > + # The master interface needs to be brought up before the slave
> > ports.
> > + ip link set eth0 up
> > + ip link set eth0.1 up
> > +
> > + # bring up the slave interfaces
> > + ip link set wan up
> > + ip link set lan1 up
> > + ip link set lan2 up
> > +
> > + # bring up the bridge
> > + ip link set dev br0 up
> > +
> > +gateway
> > +~~~~~~~
> > +
> > +.. code-block:: sh
> > +
> > + # tag traffic on CPU port
> > + ip link add link eth0 name eth0.1 type vlan id 1
> > + ip link add link eth0 name eth0.2 type vlan id 2
> > +
> > + # create bridges
> > + ip link add name br0 type bridge
> > + ip link add name br1 type bridge
>
> Likewise, I have seen claims of people telling me they used two bridge
> devices, but AFAICT this is only because the bridge master did not
> have VID 2 programmed to it, so if you did the following:
>
> bridge vlan add vid 2 dev br0 self
I tried the following:
# tag traffic on CPU port
ip link add link eth0 name eth0.1 type vlan id 1
ip link add link eth0 name eth0.2 type vlan id 2
# The master interface needs to be brought up before the slave ports.
ip link set eth0 up
ip link set eth0.1 up
ip link set eth0.2 up
# bring up the slave interfaces
ip link set wan up
ip link set lan1 up
ip link set lan2 up
# create bridge
ip link add name br0 type bridge
# activate VLAN filtering
ip link set dev br0 type bridge vlan_filtering 1
# add ports to bridges
ip link set dev wan master br0
ip link set eth0.1 master br0
ip link set dev lan1 master br0
ip link set dev lan2 master br0
# tag traffic on ports
bridge vlan add dev wan vid 2 pvid untagged
bridge vlan del dev wan vid 1
# configure the VLANs
ip addr add 192.0.2.1/30 dev eth0.2
ip addr add 192.0.2.129/25 dev br0
# bring up the bridge devices
ip link set br0 up
Maybe I got it fundamently wrong:
I try to seperate the traffic. If i bound everything to the bridge the
separation will not work out. To split it up like above on the other
hand makes it very clear (at least for me).
I have separate interfaces (here br0 and eth0.2) for the different
networks.
But I am open and grateful for any help and suggestions in that area.
> you should be able to get away with a single bridge master device, can
> you try that?
Done. See above. Works!
> Overall this is fills a lot of gaps and questions that were being
> asked on the lamobo R1 threads on various forums, thanks a lot for
> doing that!
Thx.
Regards
Bene Spranger
^ permalink raw reply
* Re: [PATCH v4] net: netfilter: Fix rpfilter dropping vrf packets by mistake
From: David Ahern @ 2019-06-28 17:04 UTC (permalink / raw)
To: Miaohe Lin, pablo, kadlec, fw, davem, kuznet, yoshfuji,
netfilter-devel, coreteam, netdev, linux-kernel
Cc: mingfangsen
In-Reply-To: <1561712803-195184-1-git-send-email-linmiaohe@huawei.com>
On 6/28/19 3:06 AM, Miaohe Lin wrote:
> diff --git a/net/ipv6/netfilter/ip6t_rpfilter.c b/net/ipv6/netfilter/ip6t_rpfilter.c
> index 6bcaf7357183..3c4a1772c15f 100644
> --- a/net/ipv6/netfilter/ip6t_rpfilter.c
> +++ b/net/ipv6/netfilter/ip6t_rpfilter.c
> @@ -55,6 +55,10 @@ static bool rpfilter_lookup_reverse6(struct net *net, const struct sk_buff *skb,
> if (rpfilter_addr_linklocal(&iph->saddr)) {
> lookup_flags |= RT6_LOOKUP_F_IFACE;
> fl6.flowi6_oif = dev->ifindex;
> + /* Set flowi6_oif for vrf devices to lookup route in l3mdev domain. */
> + } else if (netif_is_l3_master(dev) || netif_is_l3_slave(dev)) {
> + lookup_flags |= FLOWI_FLAG_SKIP_NH_OIF;
you don't need to set that flag here. It is done by the fib_rules code
as needed.
^ permalink raw reply
* Re: [PATCH 00/10] pull request for net-next: batman-adv 2019-06-27 v2
From: Sven Eckelmann @ 2019-06-28 17:01 UTC (permalink / raw)
To: b.a.t.m.a.n; +Cc: David Miller, sw, netdev, Linus Lüssing
In-Reply-To: <20190628.094905.1673194288384587104.davem@davemloft.net>
[-- Attachment #1: Type: text/plain, Size: 802 bytes --]
On Friday, 28 June 2019 18:49:05 CEST David Miller wrote:
[...]
> I think that when you have the read_lock held, RCU is not necessary in order
> to use __in6_dev_get() but I may be mistaken. Just FYI...
Problem is that the read_lock() can only be used after the __in6_dev_get()
finished sucessfully. Because the read_lock's lock is stored in the inet6_dev
object which was retrieved via __in6_dev_get. And the __in6_dev_get kerneldoc
states [1] that you either have to hold RTNL or RCU (see also the
rcu_dereference_rtnl call inside this function).
So we can only drop the rcu_read_lock when RTNL lock is held . I would guess
now that this is not the case here - Linus' might want to correct me.
Kind regards,
Sven
[1] https://lxr.missinglinkelectronics.com/linux/include/net/addrconf.h#L335
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 00/11] XDP unaligned chunk placement support
From: Björn Töpel @ 2019-06-28 16:51 UTC (permalink / raw)
To: Laatz, Kevin, Jakub Kicinski
Cc: Jonathan Lemon, netdev, ast, daniel, magnus.karlsson, bpf,
intel-wired-lan, bruce.richardson, ciara.loftus
In-Reply-To: <f0ca817a-02b4-df22-d01b-7bc07171a4dc@intel.com>
On 2019-06-28 18:19, Laatz, Kevin wrote:
> On 27/06/2019 22:25, Jakub Kicinski wrote:
>> On Thu, 27 Jun 2019 12:14:50 +0100, Laatz, Kevin wrote:
>>> On the application side (xdpsock), we don't have to worry about the user
>>> defined headroom, since it is 0, so we only need to account for the
>>> XDP_PACKET_HEADROOM when computing the original address (in the default
>>> scenario).
>> That assumes specific layout for the data inside the buffer. Some NICs
>> will prepend information like timestamp to the packet, meaning the
>> packet would start at offset XDP_PACKET_HEADROOM + metadata len..
>
> Yes, if NICs prepend extra data to the packet that would be a problem for
> using this feature in isolation. However, if we also add in support for
> in-order
> RX and TX rings, that would no longer be an issue. However, even for NICs
> which do prepend data, this patchset should not break anything that is
> currently
> working.
(Late on the ball. I'm in vacation mode.)
In your example Jakub, how would this look in XDP? Wouldn't the
timestamp be part of the metadata (xdp_md.data_meta)? Isn't
data-data_meta (if valid) <= XDP_PACKET_HEADROOM? That was my assumption.
There were some discussion on having meta data length in the struct
xdp_desc, before AF_XDP was merged, but the conclusion was that this was
*not* needed, because AF_XDP and the XDP program had an implicit
contract. If you're running AF_XDP, you also have an XDP program running
and you can determine the meta data length (and also getting back the
original buffer).
So, today in AF_XDP if XDP metadata is added, the userland application
can look it up before the xdp_desc.addr (just like regular XDP), and how
the XDP/AF_XDP application determines length/layout of the metadata i
out-of-band/not specified.
This is a bit messy/handwavy TBH, so maybe adding the length to the
descriptor *is* a good idea (extending the options part of the
xdp_desc)? Less clean though. OTOH the layout of the meta data still
need to be determined.
Björn
^ permalink raw reply
* Re: [PATCH v3 bpf-next 3/9] libbpf: add ability to attach/detach BPF program to perf event
From: Andrii Nakryiko @ 2019-06-28 16:50 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: Andrii Nakryiko, Alexei Starovoitov, Daniel Borkmann, Networking,
bpf, Kernel Team
In-Reply-To: <20190628160436.GH4866@mini-arch>
On Fri, Jun 28, 2019 at 9:04 AM Stanislav Fomichev <sdf@fomichev.me> wrote:
>
> On 06/27, Andrii Nakryiko wrote:
> > bpf_program__attach_perf_event allows to attach BPF program to existing
> > perf event hook, providing most generic and most low-level way to attach BPF
> > programs. It returns struct bpf_link, which should be passed to
> > bpf_link__destroy to detach and free resources, associated with a link.
> >
> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> > ---
> > tools/lib/bpf/libbpf.c | 58 ++++++++++++++++++++++++++++++++++++++++
> > tools/lib/bpf/libbpf.h | 3 +++
> > tools/lib/bpf/libbpf.map | 1 +
> > 3 files changed, 62 insertions(+)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index 455795e6f8af..606705f878ba 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -32,6 +32,7 @@
> > #include <linux/limits.h>
> > #include <linux/perf_event.h>
> > #include <linux/ring_buffer.h>
> > +#include <sys/ioctl.h>
> > #include <sys/stat.h>
> > #include <sys/types.h>
> > #include <sys/vfs.h>
> > @@ -3958,6 +3959,63 @@ int bpf_link__destroy(struct bpf_link *link)
> > return err;
> > }
> >
> > +struct bpf_link_fd {
> > + struct bpf_link link; /* has to be at the top of struct */
> [..]
> > + int fd; /* hook FD */
> > +};
> Any cons to storing everything in bpf_link, instead of creating a
> "subclass"? Less things to worry about.
Yes, it's not always enough to just have single FD to detach BPF
program. Check bpf_prog_detach and bpf_prog_detach2 in
tools/lib/bpf/bpf.c. For some types of attachment you have to provide
target_fd+attach_type, for some target_fd+attach_type+attach_bpf_fd.
So those two will use their own bpf_link extensions.
I haven't implemented those attachment APIs yet, but we should.
What should go into bpf_link itself is any information that's common
to any kind of attachment (e.g, "kind of attachment" itself). It's
conceivable that we might allow "casting" bpf_link into specific
variation and having extra "methods" on those. I haven't done that, as
I didn't have a need yet.
>
> > +static int bpf_link__destroy_perf_event(struct bpf_link *link)
> > +{
> > + struct bpf_link_fd *l = (void *)link;
> > + int err;
> > +
> > + if (l->fd < 0)
> > + return 0;
> > +
> > + err = ioctl(l->fd, PERF_EVENT_IOC_DISABLE, 0);
> > + close(l->fd);
> > + return err;
> > +}
> > +
> > +struct bpf_link *bpf_program__attach_perf_event(struct bpf_program *prog,
> > + int pfd)
> > +{
> > + char errmsg[STRERR_BUFSIZE];
> > + struct bpf_link_fd *link;
> > + int bpf_fd, err;
> > +
> > + bpf_fd = bpf_program__fd(prog);
> > + if (bpf_fd < 0) {
> > + pr_warning("program '%s': can't attach before loaded\n",
> > + bpf_program__title(prog, false));
> > + return ERR_PTR(-EINVAL);
> > + }
> > +
> > + link = malloc(sizeof(*link));
> > + if (!link)
> > + return ERR_PTR(-ENOMEM);
> > + link->link.destroy = &bpf_link__destroy_perf_event;
> > + link->fd = pfd;
> > +
> > + if (ioctl(pfd, PERF_EVENT_IOC_SET_BPF, bpf_fd) < 0) {
> > + err = -errno;
> > + free(link);
> > + pr_warning("program '%s': failed to attach to pfd %d: %s\n",
> > + bpf_program__title(prog, false), pfd,
> > + libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> > + return ERR_PTR(err);
> > + }
> > + if (ioctl(pfd, PERF_EVENT_IOC_ENABLE, 0) < 0) {
> > + err = -errno;
> > + free(link);
> > + pr_warning("program '%s': failed to enable pfd %d: %s\n",
> > + bpf_program__title(prog, false), pfd,
> > + libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> > + return ERR_PTR(err);
> > + }
> > + return (struct bpf_link *)link;
> > +}
> > +
> > enum bpf_perf_event_ret
> > bpf_perf_event_read_simple(void *mmap_mem, size_t mmap_size, size_t page_size,
> > void **copy_mem, size_t *copy_size,
> > diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> > index 5082a5ebb0c2..1bf66c4a9330 100644
> > --- a/tools/lib/bpf/libbpf.h
> > +++ b/tools/lib/bpf/libbpf.h
> > @@ -169,6 +169,9 @@ struct bpf_link;
> >
> > LIBBPF_API int bpf_link__destroy(struct bpf_link *link);
> >
> > +LIBBPF_API struct bpf_link *
> > +bpf_program__attach_perf_event(struct bpf_program *prog, int pfd);
> > +
> > struct bpf_insn;
> >
> > /*
> > diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> > index 3cde850fc8da..756f5aa802e9 100644
> > --- a/tools/lib/bpf/libbpf.map
> > +++ b/tools/lib/bpf/libbpf.map
> > @@ -169,6 +169,7 @@ LIBBPF_0.0.4 {
> > global:
> > bpf_link__destroy;
> > bpf_object__load_xattr;
> > + bpf_program__attach_perf_event;
> > btf_dump__dump_type;
> > btf_dump__free;
> > btf_dump__new;
> > --
> > 2.17.1
> >
^ permalink raw reply
* Re: pull-request: mac80211 2019-06-28
From: David Miller @ 2019-06-28 16:50 UTC (permalink / raw)
To: johannes; +Cc: netdev, linux-wireless
In-Reply-To: <20190628144444.25092-1-johannes@sipsolutions.net>
From: Johannes Berg <johannes@sipsolutions.net>
Date: Fri, 28 Jun 2019 16:44:43 +0200
> Just a single patch still for the current RC cycle, I debated
> whether to send a pull request at all or just ask you to apply
> the patch, but did it this way now.
>
> Please pull and let me know if there's any problem.
Either way works for me :)
Pulled, thanks.
^ permalink raw reply
* Re: [PATCH 00/10] pull request for net-next: batman-adv 2019-06-27 v2
From: David Miller @ 2019-06-28 16:49 UTC (permalink / raw)
To: sw; +Cc: netdev, b.a.t.m.a.n
In-Reply-To: <20190628135604.11581-1-sw@simonwunderlich.de>
From: Simon Wunderlich <sw@simonwunderlich.de>
Date: Fri, 28 Jun 2019 15:55:54 +0200
> here is the updated feature/cleanup pull request of batman-adv for net-next
> from yesterday. Your change suggestions have been integrated into Patch 6
> of the series, everything else is unchanged.
>
> Please pull or let me know of any problem!
Pulled, thanks Simon.
I think that when you have the read_lock held, RCU is not necessary in order
to use __in6_dev_get() but I may be mistaken. Just FYI...
^ 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