* Re: [PATCH] ib/mlx4: adjust gfp flags for DMA allocations
From: Leon Romanovsky @ 2018-07-10 10:56 UTC (permalink / raw)
To: Jan Dakinevich
Cc: Yishai Hadas, Doug Ledford, Jason Gunthorpe, Tariq Toukan,
David S. Miller, linux-rdma, linux-kernel, netdev, Denis Lunev,
Konstantin Khorenko
In-Reply-To: <1531144273-25959-1-git-send-email-jan.dakinevich@virtuozzo.com>
[-- Attachment #1: Type: text/plain, Size: 1049 bytes --]
On Mon, Jul 09, 2018 at 04:51:13PM +0300, Jan Dakinevich wrote:
> For most cases it is enough to use common GFP_KERNEL flag. However,
> there is one place with defined fallback path on unsuccessful allocation.
> In this case it would be preferred to append on first allocation attempt
> __GFP_NOWARN and __GFP_NORETRY flags to suppress possible warnings and
> to shorten the path to the fallback in a case of memory pressure.
>
> Signed-off-by: Jan Dakinevich <jan.dakinevich@virtuozzo.com>
> ---
> drivers/infiniband/hw/mlx4/cq.c | 2 +-
> drivers/infiniband/hw/mlx4/qp.c | 6 ++++--
> drivers/infiniband/hw/mlx4/srq.c | 2 +-
> drivers/net/ethernet/mellanox/mlx4/alloc.c | 12 ++++++------
> include/linux/mlx4/device.h | 2 +-
> 5 files changed, 13 insertions(+), 11 deletions(-)
>
Thanks for the patch, however we are working on the patch to avoid
second call to mlx4_buf_alloc() and it will eliminate the annoying
WARN splat.
Because it is in latest stages of internal review, NAK.
Thanks
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: [PATCH] netfilter: NFT_SOCKET don't use NF_SOCKET_IPV6 without NF_TABLES_IPV6
From: Pablo Neira Ayuso @ 2018-07-10 10:56 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Máté Eckl, Jozsef Kadlecsik, Florian Westphal,
David S. Miller, Flavio Leitner, netfilter-devel, coreteam,
Networking, Linux Kernel Mailing List
In-Reply-To: <CAK8P3a0zYmdoMdWdFDcWprZY_4w2hbMYq4--iZB=JE4x2UiscA@mail.gmail.com>
On Tue, Jul 10, 2018 at 11:10:40AM +0200, Arnd Bergmann wrote:
> On Tue, Jul 10, 2018 at 10:05 AM, Máté Eckl <ecklm94@gmail.com> wrote:
> > On Tue, Jul 10, 2018 at 10:02:27AM +0200, Máté Eckl wrote:
> >> On Mon, Jul 09, 2018 at 11:35:09PM +0200, Arnd Bergmann wrote:
> >> > It is now possible to build the nft_socket module as built-in when
> >> > NF_TABLES_IPV6 is disabled, and have NF_SOCKET_IPV6=m set manually.
> >> >
> >> > In this case, the NF_SOCKET_IPV6 functionality will be useless according
> >> > to the explanation in commit 35bf1ccecaaa ("netfilter: Kconfig: Change
> >> > IPv6 select dependencies"), but on top of that it also causes a link
> >> > error:
> >> >
> >> > net/netfilter/nft_socket.o: In function `nft_socket_eval':
> >> > nft_socket.c:(.text+0x162): undefined reference to `nf_sk_lookup_slow_v6'
> >> >
> >> > This changes the compile-time check so we don't attempt to use
> >> > the NF_SOCKET_IPV6 code when it cannot be used, and make it all
> >> > compile again. That may lead to unexpected behavior when a user
> >> > enables NF_SOCKET_IPV6 but cannot use it, but seems to be the
> >> > logical conclusion of the 35bf1ccecaaa change.
> >> >
> >> > Fixes: 35bf1ccecaaa ("netfilter: Kconfig: Change IPv6 select dependencies")
> >> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> >>
> >> I think this should be fixed in the Kconfig rather than inside the module(s).
>
> Should we revert your patch then, or do you have a better idea?
Máté, would you resubmit a new patch that addresses all the problems
that Arnd is reporting in one go?
I think it's better if we toss your original patch in the tree and
rebase, ie. take the new one that fixes all issues that Arnd is
reporting. It would be good if we can sort out this before I send the
next pull request for net-next stuff.
I was afraid of fallout like this when I saw your original patch,
kbuild is always tricky.
Please Cc Arnd, Florian and me for review.
Thanks!
^ permalink raw reply
* Re: INFO: rcu detected stall in llcp_sock_sendmsg
From: Petr Mladek @ 2018-07-10 10:55 UTC (permalink / raw)
To: Eric Dumazet
Cc: Dmitry Vyukov, syzbot, Samuel Ortiz, David Miller, linux-wireless,
netdev, LKML, Steven Rostedt, Sergey Senozhatsky, syzkaller-bugs
In-Reply-To: <6e2e4cfa-2cde-ec28-933d-1ee17c8b1da9@gmail.com>
On Mon 2018-07-09 14:05:08, Eric Dumazet wrote:
>
>
> On 07/09/2018 01:50 PM, Dmitry Vyukov wrote:
> > On Mon, Jul 9, 2018 at 10:34 PM, syzbot
> > <syzbot+e9f364d3b15ce41d8451@syzkaller.appspotmail.com> wrote:
> >> Hello,
> >>
> >> syzbot found the following crash on:
> >>
> >> HEAD commit: 1e4b044d2251 Linux 4.18-rc4
> >> git tree: upstream
> >> console output: https://syzkaller.appspot.com/x/log.txt?x=1414c2c2400000
> >> kernel config: https://syzkaller.appspot.com/x/.config?x=25856fac4e580aa7
> >> dashboard link: https://syzkaller.appspot.com/bug?extid=e9f364d3b15ce41d8451
> >> compiler: gcc (GCC) 8.0.1 20180413 (experimental)
> >>
> >> Unfortunately, I don't have any reproducer for this crash yet.
> >>
> >> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> >> Reported-by: syzbot+e9f364d3b15ce41d8451@syzkaller.appspotmail.com
> >
> > Looks like the problem is actually in nfc, so +nfc maintainers.
>
> Note this issue was discussed before, maybe we should patch NFC without waiting for nfc maintainer.
Do you have any particular solution in mind, please? See below.
> ----------------------------------------------------
>
> On 06/25/2018 10:12 PM, Sergey Senozhatsky wrote:
> > On (06/26/18 07:07), Dmitry Vyukov wrote:
> > [..]
> >>> #include <net/nfc/nfc.h>
> >>> @@ -755,7 +756,8 @@ int nfc_llcp_send_ui_frame(struct nfc_llcp_sock *sock, u8 ssap, u8 dsap,
> >>> pdu = nfc_alloc_send_skb(sock->dev, &sock->sk, MSG_DONTWAIT,
> >>> frag_len + LLCP_HEADER_SIZE, &err);
> >>> if (pdu == NULL) {
> >>> - pr_err("Could not allocate PDU\n");
> >>> + pr_err_ratelimited("Could not allocate PDU\n");
> >>> + cond_resched();
> >>> continue;
> >>> }
> >>
> >>
> >> But this thread is still in an infinite (unkillable?) loop? If yes, we
> >> are waiting for the next syzbot report
> >
> > The loop is still infinite, correct, but we have a preemption point now.
> > Sure, net people can come with a much better solution, I'll be happy to
> > scratch my patch.
> >
>
> This can not be the right solution, think about current thread being real time,
> cond_resched() might be a nop.
>
> We should probably not loop at all, or not use MSG_DONTWAIT.
These two solutions look promising. But they both need to
get reviewed by someone familiar with the code.
On one hand, nfc_llcp_send_ui_frame() already returns some errors
before sending anything. But I am not sure how to deal with situation
when a fragment of the message has already been sent.
Best Regards,
Petr
^ permalink raw reply
* Re: [PATCH bpf] bpf: fix ldx in ld_abs rewrite for large offsets
From: Mark Rutland @ 2018-07-10 10:14 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: alexei.starovoitov, netdev
In-Reply-To: <1aaf318d0b40fc471a0d1705efd8c69d98fbf866.1531175919.git.daniel@iogearbox.net>
On Tue, Jul 10, 2018 at 12:43:22AM +0200, Daniel Borkmann wrote:
> Mark reported that syzkaller triggered a KASAN detected slab-out-of-bounds
> bug in ___bpf_prog_run() with a BPF_LD | BPF_ABS word load at offset 0x8001.
> After further investigation it became clear that the issue was the
> BPF_LDX_MEM() which takes offset as an argument whereas it cannot encode
> larger than S16_MAX offsets into it. For this synthetical case we need to
> move the full address into tmp register instead and do the LDX without
> immediate value.
>
> Fixes: e0cea7ce988c ("bpf: implement ld_abs/ld_ind in native bpf")
> Reported-by: syzbot <syzkaller@googlegroups.com>
> Reported-by: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> ---
> net/core/filter.c | 16 +++++++++++++---
> 1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 5fa66a3..a13f5b1 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -459,11 +459,21 @@ static bool convert_bpf_ld_abs(struct sock_filter *fp, struct bpf_insn **insnp)
> (!unaligned_ok && offset >= 0 &&
> offset + ip_align >= 0 &&
> offset + ip_align % size == 0))) {
> + bool ldx_off_ok = offset <= S16_MAX;
> +
Given offset is a (signed) int, is it possible for that to be a negative
value less than S16_MIN? ... or is that ruled out elsewhere?
Thanks,
Mark.
> *insn++ = BPF_MOV64_REG(BPF_REG_TMP, BPF_REG_H);
> *insn++ = BPF_ALU64_IMM(BPF_SUB, BPF_REG_TMP, offset);
> - *insn++ = BPF_JMP_IMM(BPF_JSLT, BPF_REG_TMP, size, 2 + endian);
> - *insn++ = BPF_LDX_MEM(BPF_SIZE(fp->code), BPF_REG_A, BPF_REG_D,
> - offset);
> + *insn++ = BPF_JMP_IMM(BPF_JSLT, BPF_REG_TMP,
> + size, 2 + endian + (!ldx_off_ok * 2));
> + if (ldx_off_ok) {
> + *insn++ = BPF_LDX_MEM(BPF_SIZE(fp->code), BPF_REG_A,
> + BPF_REG_D, offset);
> + } else {
> + *insn++ = BPF_MOV64_REG(BPF_REG_TMP, BPF_REG_D);
> + *insn++ = BPF_ALU64_IMM(BPF_ADD, BPF_REG_TMP, offset);
> + *insn++ = BPF_LDX_MEM(BPF_SIZE(fp->code), BPF_REG_A,
> + BPF_REG_TMP, 0);
> + }
> if (endian)
> *insn++ = BPF_ENDIAN(BPF_FROM_BE, BPF_REG_A, size * 8);
> *insn++ = BPF_JMP_A(8);
> --
> 2.9.5
>
^ permalink raw reply
* Re: [PATCH net v2 4/5] net/ipv6: propagate net.ipv6.conf.all.addr_gen_mode to devices
From: Sabrina Dubroca @ 2018-07-10 10:13 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, Jiri Pirko, Felix Jia
In-Reply-To: <9e3ad74a-2321-9893-dbcb-5dec1d7ef054@gmail.com>
2018-07-09, 11:24:49 -0600, David Ahern wrote:
> On 7/9/18 4:25 AM, Sabrina Dubroca wrote:
> > This aligns the addr_gen_mode sysctl with the expected behavior of the
> > "all" variant.
> >
> > Fixes: d35a00b8e33d ("net/ipv6: allow sysctl to change link-local address generation mode")
> > Suggested-by: David Ahern <dsahern@gmail.com>
> > Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
> > ---
> > net/ipv6/addrconf.c | 12 ++++++++++++
> > 1 file changed, 12 insertions(+)
> >
> > diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> > index e89bca83e0e4..1659a6b3cf42 100644
> > --- a/net/ipv6/addrconf.c
> > +++ b/net/ipv6/addrconf.c
> > @@ -5926,6 +5926,18 @@ static int addrconf_sysctl_addr_gen_mode(struct ctl_table *ctl, int write,
> > idev->cnf.addr_gen_mode = new_val;
> > addrconf_dev_config(idev->dev);
> > }
> > + } else if (&net->ipv6.devconf_all->addr_gen_mode == ctl->data) {
> > + struct net_device *dev;
> > +
> > + net->ipv6.devconf_dflt->addr_gen_mode = new_val;
> > + for_each_netdev(net, dev) {
> > + idev = __in6_dev_get(dev);
> > + if (idev &&
> > + idev->cnf.addr_gen_mode != new_val) {
> > + idev->cnf.addr_gen_mode = new_val;
> > + addrconf_dev_config(idev->dev);
>
> This call is adding a new LL address without removing the previous one:
>
> # ip -6 addr sh dev eth2
> 4: eth2: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 state UP qlen 1000
> inet6 2001:db8:2::4/64 scope global
> valid_lft forever preferred_lft forever
> inet6 fe80::e0:f9ff:fe45:6480/64 scope link
> valid_lft forever preferred_lft forever
>
> # sysctl -w net.ipv6.conf.eth2.addr_gen_mode=3
> net.ipv6.conf.eth2.addr_gen_mode = 3
>
> # ip -6 addr sh dev eth2
> 4: eth2: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 state UP qlen 1000
> inet6 2001:db8:2::4/64 scope global
> valid_lft forever preferred_lft forever
> inet6 fe80::bc31:8009:270d:e019/64 scope link stable-privacy
> valid_lft forever preferred_lft forever
> inet6 fe80::e0:f9ff:fe45:6480/64 scope link
> valid_lft forever preferred_lft forever
Yes. That's also what will happen with global addresses, once the next
RA is received: a new address corresponding to the new generation mode
will be added, and the old one isn't removed.
I think that was the expected behavior of d35a00b8e33d, but since it
never actually worked... OTOH, the netlink attribute only sets
idev->cnf.addr_gen_mode and doesn't add the new LL address (not until
a DOWN/UP cycle), which I personally find surprising. If I set the
mode to random or stable_secret, I would expect the privacy address to
show up without having to take the device down and then up.
I think removing the previous address immediately would break things
(and the user wouldn't expect an address to disappear that way, since
they're not explicitly asking for it to be removed), but I guess we
could play games with the lifetimes (reduce the lifetime of the old
address from forever to some limit). That limit would need to be
configurable I think, and I would rather target that change for
net-next.
--
Sabrina
^ permalink raw reply
* Re: [PATCH] netfilter: NFT_SOCKET don't use NF_SOCKET_IPV6 without NF_TABLES_IPV6
From: Máté Eckl @ 2018-07-10 9:51 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
David S. Miller, Flavio Leitner, netfilter-devel, coreteam,
Networking, Linux Kernel Mailing List
In-Reply-To: <CAK8P3a0zYmdoMdWdFDcWprZY_4w2hbMYq4--iZB=JE4x2UiscA@mail.gmail.com>
On Tue, Jul 10, 2018 at 11:10:40AM +0200, Arnd Bergmann wrote:
> On Tue, Jul 10, 2018 at 10:05 AM, Máté Eckl <ecklm94@gmail.com> wrote:
> > On Tue, Jul 10, 2018 at 10:02:27AM +0200, Máté Eckl wrote:
> >> On Mon, Jul 09, 2018 at 11:35:09PM +0200, Arnd Bergmann wrote:
> >> > It is now possible to build the nft_socket module as built-in when
> >> > NF_TABLES_IPV6 is disabled, and have NF_SOCKET_IPV6=m set manually.
> >> >
> >> > In this case, the NF_SOCKET_IPV6 functionality will be useless according
> >> > to the explanation in commit 35bf1ccecaaa ("netfilter: Kconfig: Change
> >> > IPv6 select dependencies"), but on top of that it also causes a link
> >> > error:
> >> >
> >> > net/netfilter/nft_socket.o: In function `nft_socket_eval':
> >> > nft_socket.c:(.text+0x162): undefined reference to `nf_sk_lookup_slow_v6'
> >> >
> >> > This changes the compile-time check so we don't attempt to use
> >> > the NF_SOCKET_IPV6 code when it cannot be used, and make it all
> >> > compile again. That may lead to unexpected behavior when a user
> >> > enables NF_SOCKET_IPV6 but cannot use it, but seems to be the
> >> > logical conclusion of the 35bf1ccecaaa change.
> >> >
> >> > Fixes: 35bf1ccecaaa ("netfilter: Kconfig: Change IPv6 select dependencies")
> >> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> >>
> >> I think this should be fixed in the Kconfig rather than inside the module(s).
>
> Should we revert your patch then, or do you have a better idea?
>
> >> I did some investigation and it turns out that you missed a circumstance. This
> >> link error occures only if NFT_SOCKET=y && NF_SOCKET_IPV6=m && NF_TABLES_IPV6=y
> >> (cannot be m here if NFT_SOCKET is y).
>
> No, if NF_TABLES_IPV6=y the problem cannot happen, since NFT_SOCKET then
> selects NF_SOCKET_IPV6=y as well. Before your patch, it would always select
> NF_SOCKET_IPV6 when it could, so it worked in all configurations.
Sorry I wanted to write NF_TABLES_IPV6=n...
So: NFT_SOCKET=y && NF_SOCKET_IPV6=m && NF_TABLES_IPV6=n causes linkage error.
NFT_SOCKET=m && NF_SOCKET_IPV6=m && NF_TABLES_IPV6=n compiles fine.
> >> And probably the same with
> >> iptables-related modules. Probably this possibility should be eliminated.
> >
> > NF_TPROXY_IPV6 might be in the same situation.
>
> I tried coming up with a combination that is broken for NF_TPROXY_IPV6=m
> but could not. From what I can see with
>
> config NETFILTER_XT_TARGET_TPROXY
> tristate '"TPROXY" target transparent proxying support'
> depends on IP6_NF_IPTABLES || IP6_NF_IPTABLES=n
> select NF_TPROXY_IPV6 if IP6_NF_IPTABLES
>
> and
>
> #if IS_ENABLED(CONFIG_IP6_NF_IPTABLES)
>
> inside of net/netfilter/xt_TPROXY.c, there is no way we can end up with
> xt_TPROXY calling into the nf_tproxy_ipv6 loadable module from
> a built-in context. This is the same approach I used in my patch,
> just with IP6_NF_IPTABLES instead of NF_SOCKET_IPV6, in both
> the Kconfig dependency and the module.
Right, I see your point. I have an alternative solution which seems more robust
to me, but I might be overthinking this situation.
So
Accepted-by: Máté Eckl <ecklm94@gmail.com>
> Arnd
^ permalink raw reply
* RE: [PATCH net-next] qed: Add srq core support for RoCE and iWARP
From: Bason, Yuval @ 2018-07-10 9:20 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: davem@davemloft.net, netdev@vger.kernel.org, dledford@redhat.com,
linux-rdma@vger.kernel.org, Kalderon, Michal, Elior, Ariel
In-Reply-To: <20180709164204.GB13496@ziepe.ca>
> From: Jason Gunthorpe [mailto:jgg@ziepe.ca]
> Sent: Monday, July 9, 2018 7:42 PM
>
> On Wed, May 30, 2018 at 04:11:37PM +0300, Yuval Bason wrote:
> > This patch adds support for configuring SRQ and provides the necessary
> > APIs for rdma upper layer driver (qedr) to enable the SRQ feature.
> >
> > Signed-off-by: Michal Kalderon <michal.kalderon@cavium.com>
> > Signed-off-by: Ariel Elior <ariel.elior@cavium.com>
> > Signed-off-by: Yuval Bason <yuval.bason@cavium.com>
> > ---
> > drivers/net/ethernet/qlogic/qed/qed_cxt.c | 5 +-
> > drivers/net/ethernet/qlogic/qed/qed_cxt.h | 1 +
> > drivers/net/ethernet/qlogic/qed/qed_hsi.h | 2 +
> > drivers/net/ethernet/qlogic/qed/qed_iwarp.c | 23 ++++
> > drivers/net/ethernet/qlogic/qed/qed_main.c | 2 +
> > drivers/net/ethernet/qlogic/qed/qed_rdma.c | 179
> +++++++++++++++++++++++++++-
> > drivers/net/ethernet/qlogic/qed/qed_rdma.h | 2 +
> > drivers/net/ethernet/qlogic/qed/qed_roce.c | 17 ++-
> > include/linux/qed/qed_rdma_if.h | 12 +-
> > 9 files changed, 235 insertions(+), 8 deletions(-)
>
> Is this a pre-requisite for your related RDMA patches?
>
Yes, but this was already accepted to net-next in previous cycle and is part of the rdma-next tree:
https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git/commit/drivers/net/ethernet/qlogic/qed?h=for-next&id=39dbc646fd2c67ee9b71450ce172cbd714d4e7fb
> If yes, are you proposing that this patch should go via the RDMA tree?
>
> Jason
^ permalink raw reply
* Re: [BUG] mlx5 have problems with ipv4-ipv6 tunnels in linux 4.4
From: Konstantin Khlebnikov @ 2018-07-10 9:19 UTC (permalink / raw)
To: Saeed Mahameed
Cc: netdev, Saeed Mahameed, Or Gerlitz, Tariq Toukan, Gal Pressman
In-Reply-To: <CALzJLG8wWzfddbnbsjfMKPcsP-byCcKeWmmBYWvpfbDX1m6yqg@mail.gmail.com>
On 10.07.2018 01:31, Saeed Mahameed wrote:
> On Tue, Jul 3, 2018 at 10:45 PM, Konstantin Khlebnikov
> <khlebnikov@yandex-team.ru> wrote:
>> I'm seeing problems with tunnelled traffic with Mellanox Technologies
>> MT27710 Family [ConnectX-4 Lx] using vanilla driver from linux 4.4.y
>>
>> Packets with payload bigger than 116 bytes are not exmited.
>> Smaller packets and normal ipv6 works fine.
>>
>
> Hi Konstantin,
>
> Is this true for all ipv6 traffic or just ipv4-ipv6 tunnels ?
>
> what is the skb_network_offset(skb) for such packet ?
>
>> In linux 4.9, 4.14 and out-of-tree driver everything seems fine for now.
>> It's hard to guess or bisect commit: there are a lot of changes and
>> something wrong with driver or swiotlb in 4.7..4.8.
>> 4.6 is affected too - so this should be something between 4.6 and 4.9
>>
>> Probably this case was fixed indirectly by adding some kind of offload and
>> non-offloaded path is still broken.
>> Please give me a hint: which commit could it be.
>>
>
> I suspect it works in a newer kernel since we introduced on 4.7/4.8:
Yes, this works. Thank you.
Problem was with VLAN rather than tunnel.
This hunk from first patch is enough:
-#define MLX5E_MIN_INLINE ETH_HLEN
+#define MLX5E_MIN_INLINE (ETH_HLEN + VLAN_HLEN)
In my case full data path looks like
( tcp -> ipip6 -> veth ) -> netns-to-host -> ( veth -> vlan at mlx5 )
Tunnelled traffic also goes to vlan, while most of other traffic goes
through non-tagged interface and worked fine.
max_inline is 226 so (226 - vlan - ethernet - ipv6 - ipv4 - tcp)
leaves exactly 116 bytes for payload.
>
> commit e3a19b53cbb0e6738b7a547f262179065b72e3fa
> Author: Matthew Finlay <matt@mellanox.com>
> Date: Thu Jun 30 17:34:47 2016 +0300
>
> net/mlx5e: Copy all L2 headers into inline segment
>
> ConnectX4-Lx uses an inline wqe mode that currently defaults to
> requiring the entire L2 header be included in the wqe.
> This patch fixes mlx5e_get_inline_hdr_size() to account for
> all L2 headers (VLAN, QinQ, etc) using skb_network_offset(skb).
>
> Fixes: e586b3b0baee ("net/mlx5: Ethernet Datapath files")
> Signed-off-by: Matthew Finlay <matt@mellanox.com>
> Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
>
>
>
> commit ae76715d153e33c249b6850361e4d8d775388b5a
> Author: Hadar Hen Zion <hadarh@mellanox.com>
> Date: Sun Jul 24 16:12:39 2016 +0300
>
> net/mlx5e: Check the minimum inline header mode before xmit
>
> and then some fixes on top of it, such as:
>
> commit f600c6088018d1dbc5777d18daa83660f7ea4a64
> Author: Eran Ben Elisha <eranbe@mellanox.com>
> Date: Thu Jan 25 11:18:09 2018 +0200
>
> net/mlx5e: Verify inline header size do not exceed SKB linear size
>
>
> anyhow, can you try the above patches one by one on 4.4.y and see if it helps ?
>
>
> Thanks,
> Saeed
>
^ permalink raw reply
* RE: [net-next,v3] tcp: Improve setsockopt() TCP_USER_TIMEOUT accuracy
From: David Laight @ 2018-07-10 9:17 UTC (permalink / raw)
To: 'Jon Maxwell', davem@davemloft.net
Cc: edumazet@google.com, eric.dumazet@gmail.com, ncardwell@google.com,
kuznet@ms2.inr.ac.ru, yoshfuji@linux-ipv6.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
jmaxwell@redhat.com
In-Reply-To: <20180710065147.27647-1-jmaxwell37@gmail.com>
From: Jon Maxwell
> Sent: 10 July 2018 07:52
...
> +static __u32 tcp_clamp_rto_to_user_timeout(struct sock *sk)
> +{
> + struct inet_connection_sock *icsk = inet_csk(sk);
> + __u32 elapsed, user_timeout;
> + u32 start_ts;
> +
> + start_ts = tcp_retransmit_stamp(sk);
> + if (!icsk->icsk_user_timeout || !start_ts)
> + return icsk->icsk_rto;
> + elapsed = tcp_time_stamp(tcp_sk(sk)) - start_ts;
> + user_timeout = jiffies_to_msecs(icsk->icsk_user_timeout);
> + if (elapsed >= user_timeout)
> + return 1; /* user timeout has passed; fire ASAP */
> + else
> + return (icsk->icsk_rto < msecs_to_jiffies(user_timeout - elapsed)) ?
> + icsk->icsk_rto : msecs_to_jiffies(user_timeout - elapsed);
You've not looked at what the above actually does - something like:
msecs_to_jiffies() probably rounds up, the old code rounded down
so dividing by HZ is probably right.
int new_tmo = icsk->icsk_user_timeout - (tcp_time_stamp(tcp_sk(sk)) - start_ts)/HZ;
if (new_tmo > (int) icsk->icsk_rto;
return icsk->icsk_rto;
return new_tmo <= 0 ? 1 : new_tmo;
}
^ permalink raw reply
* Re: [PATCH] netfilter: NFT_SOCKET don't use NF_SOCKET_IPV6 without NF_TABLES_IPV6
From: Arnd Bergmann @ 2018-07-10 9:10 UTC (permalink / raw)
To: Máté Eckl
Cc: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
David S. Miller, Flavio Leitner, netfilter-devel, coreteam,
Networking, Linux Kernel Mailing List
In-Reply-To: <20180710080538.d7xqpjdvpksfrx6o@sch.bme.hu>
On Tue, Jul 10, 2018 at 10:05 AM, Máté Eckl <ecklm94@gmail.com> wrote:
> On Tue, Jul 10, 2018 at 10:02:27AM +0200, Máté Eckl wrote:
>> On Mon, Jul 09, 2018 at 11:35:09PM +0200, Arnd Bergmann wrote:
>> > It is now possible to build the nft_socket module as built-in when
>> > NF_TABLES_IPV6 is disabled, and have NF_SOCKET_IPV6=m set manually.
>> >
>> > In this case, the NF_SOCKET_IPV6 functionality will be useless according
>> > to the explanation in commit 35bf1ccecaaa ("netfilter: Kconfig: Change
>> > IPv6 select dependencies"), but on top of that it also causes a link
>> > error:
>> >
>> > net/netfilter/nft_socket.o: In function `nft_socket_eval':
>> > nft_socket.c:(.text+0x162): undefined reference to `nf_sk_lookup_slow_v6'
>> >
>> > This changes the compile-time check so we don't attempt to use
>> > the NF_SOCKET_IPV6 code when it cannot be used, and make it all
>> > compile again. That may lead to unexpected behavior when a user
>> > enables NF_SOCKET_IPV6 but cannot use it, but seems to be the
>> > logical conclusion of the 35bf1ccecaaa change.
>> >
>> > Fixes: 35bf1ccecaaa ("netfilter: Kconfig: Change IPv6 select dependencies")
>> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>>
>> I think this should be fixed in the Kconfig rather than inside the module(s).
Should we revert your patch then, or do you have a better idea?
>> I did some investigation and it turns out that you missed a circumstance. This
>> link error occures only if NFT_SOCKET=y && NF_SOCKET_IPV6=m && NF_TABLES_IPV6=y
>> (cannot be m here if NFT_SOCKET is y).
No, if NF_TABLES_IPV6=y the problem cannot happen, since NFT_SOCKET then
selects NF_SOCKET_IPV6=y as well. Before your patch, it would always select
NF_SOCKET_IPV6 when it could, so it worked in all configurations.
>> And probably the same with
>> iptables-related modules. Probably this possibility should be eliminated.
>
> NF_TPROXY_IPV6 might be in the same situation.
I tried coming up with a combination that is broken for NF_TPROXY_IPV6=m
but could not. From what I can see with
config NETFILTER_XT_TARGET_TPROXY
tristate '"TPROXY" target transparent proxying support'
depends on IP6_NF_IPTABLES || IP6_NF_IPTABLES=n
select NF_TPROXY_IPV6 if IP6_NF_IPTABLES
and
#if IS_ENABLED(CONFIG_IP6_NF_IPTABLES)
inside of net/netfilter/xt_TPROXY.c, there is no way we can end up with
xt_TPROXY calling into the nf_tproxy_ipv6 loadable module from
a built-in context. This is the same approach I used in my patch,
just with IP6_NF_IPTABLES instead of NF_SOCKET_IPV6, in both
the Kconfig dependency and the module.
Arnd
^ permalink raw reply
* Re: [libvirt] opening tap devices that are created in a container
From: Daniel P. Berrangé @ 2018-07-10 8:47 UTC (permalink / raw)
To: Jason Baron
Cc: fabiand, libvir-list, netdev, Roman Mohr, ebiederm,
Martin Kletzander, davem
In-Reply-To: <6f5f40b6-3637-c7a9-44f8-81352ece2bef@akamai.com>
On Mon, Jul 09, 2018 at 05:00:49PM -0400, Jason Baron wrote:
>
>
> On 07/08/2018 02:01 AM, Martin Kletzander wrote:
> > On Thu, Jul 05, 2018 at 06:24:20PM +0200, Roman Mohr wrote:
> >> On Thu, Jul 5, 2018 at 4:20 PM Jason Baron <jbaron@akamai.com> wrote:
> >>
> >>> Hi,
> >>>
> >>> Opening tap devices, such as macvtap, that are created in containers is
> >>> problematic because the interface for opening tap devices is via
> >>> /dev/tapNN and devtmpfs is not typically mounted inside a container as
> >>> its not namespace aware. It is possible to do a mknod() in the
> >>> container, once the tap devices are created, however, since the tap
> >>> devices are created dynamically its not possible to apriori allow access
> >>> to certain major/minor numbers, since we don't know what these are going
> >>> to be. In addition, its desirable to not allow the mknod capability in
> >>> containers. This behavior, I think is somewhat inconsistent with the
> >>> tuntap driver where one can create tuntap devices inside a container by
> >>> first opening /dev/net/tun and then using them by supplying the tuntap
> >>> device name via the ioctl(TUNSETIFF). And since TUNSETIFF validates the
> >>> network namespace, one is limited to opening network devices that belong
> >>> to your current network namespace.
> >>>
> >>> Here are some options to this issue, that I wanted to get feedback
> >>> about, and just wondering if anybody else has run into this.
> >>>
> >>> 1)
> >>>
> >>> Don't create the tap device, such as macvtap in the container. Instead,
> >>> create the tap device outside of the container and then move it into the
> >>> desired container network namespace. In addition, do a mknod() for the
> >>> corresponding /dev/tapNN device from outside the container before doing
> >>> chroot().
> >>>
> >>> This solution still doesn't allow tap devices to be created inside the
> >>> container. Thus, in the case of kubevirt, which runs libvirtd inside of
> >>> a container, it would mean changing libvirtd to open existing tap
> >>> devices (as opposed to the current behavior of creating new ones). This
> >>> would not require any kernel changes, but as mentioned seems
> >>> inconsistent with the tuntap interface.
> >>>
> >>
> >> For KubeVirt, apart from how exactly the device ends up in the
> >> container, I
> >> would want to pursue a way where all network preparations which require
> >> privileges happens from a privileged process *outside* of the container.
> >> Like CNI solutions do it. They run outside, have privileges and then
> >> create
> >> devices in the right network/mount namespace or move them there. The
> >> final
> >> goal for KubeVirt is that our pod with the qemu process is completely
> >> unprivileged and privileged setup happens from outside.
> >>
> >> As a consequence, and depending on which route Dan pursues with the
> >> restructured libvirt, I would assume that either a privileged
> >> libvirtd-part
> >> outside of containers creates the devices by entering the right
> >> namespaces,
> >> or that libvirt in the container can consume pre-created tun/tap devices,
> >> like qemu.
> >>
> >
> > That would be nice, but as far as I understand there will always be a
> > need for
> > some privileges if you want to use a tap device. It's nice that CNI
> > does that
> > and all the containers can run unprivileged, but that's because they do
> > not open
> > the tap device and they do not do any privileged operations on it. But
> > QEMU
> > needs to. So the only way would be passing an opened fd to the
> > container or
> > opening the tap device there and making the fd usable for one process in
> > the
> > container. Is this already supported for some type of containers in
> > some way?
> >
> > Martin
>
> Hi,
>
> So another option here call it #3 is to pass open fds via unix sockets.
> If there are privileged operations that QEMU is trying to do with the fd
> though, how will opening it first and then passing it to an unprivileged
> QEMU address that? Is the opener doing those operations first?
>From libvirt's POV, it would be preferrable to be able to open the
macvtap device by name inside the container, rather than having to
accept a pre-opened FD from the application.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
^ permalink raw reply
* Re: opening tap devices that are created in a container
From: Daniel P. Berrangé @ 2018-07-10 8:46 UTC (permalink / raw)
To: Jason Baron
Cc: netdev@vger.kernel.org, David S. Miller, libvir-list, rmohr,
Fabian Deutsch, Eric W. Biederman
In-Reply-To: <72dfae4e-876a-729e-6dc6-61219f8d0294@akamai.com>
On Mon, Jul 09, 2018 at 04:56:04PM -0400, Jason Baron wrote:
>
>
> On 07/05/2018 12:10 PM, Daniel P. Berrangé wrote:
> > On Thu, Jul 05, 2018 at 10:20:16AM -0400, Jason Baron wrote:
> >> Hi,
> >>
> >> Opening tap devices, such as macvtap, that are created in containers is
> >> problematic because the interface for opening tap devices is via
> >> /dev/tapNN and devtmpfs is not typically mounted inside a container as
> >> its not namespace aware. It is possible to do a mknod() in the
> >> container, once the tap devices are created, however, since the tap
> >> devices are created dynamically its not possible to apriori allow access
> >> to certain major/minor numbers, since we don't know what these are going
> >> to be. In addition, its desirable to not allow the mknod capability in
> >> containers. This behavior, I think is somewhat inconsistent with the
> >> tuntap driver where one can create tuntap devices inside a container by
> >> first opening /dev/net/tun and then using them by supplying the tuntap
> >> device name via the ioctl(TUNSETIFF). And since TUNSETIFF validates the
> >> network namespace, one is limited to opening network devices that belong
> >> to your current network namespace.
> >>
> >> Here are some options to this issue, that I wanted to get feedback
> >> about, and just wondering if anybody else has run into this.
> >>
> >> 1)
> >>
> >> Don't create the tap device, such as macvtap in the container. Instead,
> >> create the tap device outside of the container and then move it into the
> >> desired container network namespace. In addition, do a mknod() for the
> >> corresponding /dev/tapNN device from outside the container before doing
> >> chroot().
> >>
> >> This solution still doesn't allow tap devices to be created inside the
> >> container. Thus, in the case of kubevirt, which runs libvirtd inside of
> >> a container, it would mean changing libvirtd to open existing tap
> >> devices (as opposed to the current behavior of creating new ones). This
> >> would not require any kernel changes, but as mentioned seems
> >> inconsistent with the tuntap interface.
> >
> > Presumably the /dev/tapNN device name also changes when you rename
> > the tap device interface using SIOCSIFNAME ?
> >
>
> I don't think so. the NN is the ifindex of the device- changing the
> device name does not affect the ifindex.
Ah right that makes sense.
> > eg if it was /dev/tap24 in the host and you called SIOCSIFNAME(eth0)
> > when moving it into the container, it would be /dev/eth0 inside the
> > container ?
> >
>
> When moving it into the container the ifindex can change since the
> ifindex range is per-namespace (not global).
Oh thats interesting, I hadn't realized that.
> > Anyway, given that this /dev/tapNN approach is what exists today,
> > libvirt will likely want to implement support for this regardless
> > in order to support existing kernels.
>
> Ok, in this case whatever created the tap device outside of the
> container would pass the name of the device to libvirt and make sure
> that the /dev/tapNN device was setup correctly in the container. I
> believe this differs from how libvirt works today in that libvirt would
> need to be modified to open an existing device (I think it currently
> always creates new ones).
Libvirt can use a pre-created TAP device today, but not a pre-created
MACVTAP, so supporting the latter is new code for us no matter what.
> > One slight complication with either of the solutions above is that
> > libvirt won't know whether it is given a TAP or a MACVTAP device.
> > It'll only be given the device name. So with code today we would
> > probably have to first try /dev/tapNNN and if that doesn't exist
> > then try /dev/net/tun with TUNSETIFF.
> >
>
> hmmm. doesn't libvirt make this distinction today?
No need to make the distinction yet, since we only support pre-created
TAP devices right now. In cases where we create the devices ourselves,
we already know what is what.
> > If adding a new /dev/net/tap, something could seemlessy accept
> > either a TAP or MACTAP nic name would be nice.
> >
> >
>
> I think if we added a new ioctl() as I proposed it could accept either
> type of nic.
ok that would be nice.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply
* Re: [V9fs-developer] [PATCH] p9_check_errors() validate PDU length
From: Dominique Martinet @ 2018-07-10 8:38 UTC (permalink / raw)
To: Tomas Bortoli
Cc: ericvh, rminnich, lucho, netdev, linux-kernel, syzkaller,
v9fs-developer, davem, Al Viro
In-Reply-To: <ba4e159c-a78e-1d42-ca11-d4c8897a9a83@gmail.com>
Tomas Bortoli wrote on Tue, Jul 10, 2018:
> > This however gets more complicated once you start factoring in that
> > change I suggested about p9_parse_header not setting size (and checking
> > size) because trans_fd relies on it; so I'm not sure how we should
> > proceed.
>
> Mmh, me neither. I don't see where the *actual* PDU length is stored.
That's precisely the problem, none of the transports actually write in
the pdu how much has been read.
Instead, we blindly trust how much the client says it has written and
p9_parse_header will write pdu->size which is then used as the 'actual'
PDU length.
With the three lines I added in each file all transports will store the
PDU length in pdu->size so my following suggestion would be to just
check that in p9_parse_header the length read from the packet match that
of the received data and flag any packet where this is wrong as invalid.
The only problem with that is that TCP will use p9_parse_header with a
partial packet to know how much it needs to read, so that modification
needs actual testing (hence my question below) as it is more intrusive
than a simple boundary check.
I'd suggest implementing some logic like already here with the if
(pdu->size == 0) -- only check if pdu->size was previously set, and set
it if it wasn't... But feel free to try something else.
With that done I don't expect more problems but if I do it there's
little point in making you do it as well ;)
> > Do you have a working 9p tcp server to test changes are valid, or are
> > you only working off the syzbot reproducer?
>
> No, I was just using the reproducer to test.
>
> > In the first place, are you willing to take the time to do that bigger
> > fix?
>
> Yes.
Ok, let's get you a working setup for TCP then.
You need to install a 9p server, the two I'm aware of are diod and
nfs-ganesha (I'm using ganesha); once you have a working config you can
mount with something like this:
mount -t 9p -o aname=path,trans=fd <server-ip> <mount-point>
Once you have that you should be able to fiddle with things until it
works as expected.
(for virtio I use qemu, that's probably easy to test as well; for RDMA
you can use rxe to setup a virtual interface (with rxe_cfg) and then use
either diod or ganesha again but the setup is more complicated so it's
OK to leave that aside for now)
> For me both ways are good. Signed-off-by will be good.
> You know for sure more than me about 9p as I started delving it for the
> first time yesterday. I can also make and test a patch but I'll need to
> understand more about it. Let me know if you find out more.
There's a saying about giving a fish or teaching how to fish, so let's
have you try if you're motivated :)
I don't think much is left if you can sew the pieces together, the most
difficult part will probably to set the test environment up if you want
to do this properly.
Please ask if you run into any trouble,
--
Dominique Martinet | Asmadeus
^ permalink raw reply
* Re: [V9fs-developer] [PATCH] Integer underflow in pdu_read()
From: Tomas Bortoli @ 2018-07-10 8:27 UTC (permalink / raw)
To: piaojun, ericvh, rminnich, lucho
Cc: netdev, linux-kernel, syzkaller, v9fs-developer, davem
In-Reply-To: <5B440B6A.9090000@huawei.com>
Hi Jun,
Intuitively, if you have a packet of size x and you read at an offset y,
when y>x you are off the packet. That's an out out bound read.
In this specific code when offset > size, the available length
estimation will fail as there will be an underflow resulting from
offset-size (it'll give a big big number) that breaks the out-of-bound
control put in place (if offset-size is a big big number, the asked size
to read will be probably smaller and therefore allowed).
These definitions might help:
https://cwe.mitre.org/data/definitions/787.html
https://cwe.mitre.org/data/definitions/125.html
Tomas
> Hi Tomas,
>
> It looks like pdu->size should always be greater than pdu->offset, right?
> My question may be very easy for you, please help explaining.
>
> Thanks,
> Jun
>
> On 2018/7/10 3:26, Tomas Bortoli wrote:
>> The pdu_read() function suffers from an integer underflow.
>> When pdu->offset is greater than pdu->size, the length calculation will have
>> a wrong result, resulting in an out-of-bound read.
>> This patch modifies also pdu_write() in the same way to prevent the same
>> issue from happening there and for consistency.
>>
>> Signed-off-by: Tomas Bortoli <tomasbortoli@gmail.com>
>> Reported-by: syzbot+65c6b72f284a39d416b4@syzkaller.appspotmail.com
>> ---
>> net/9p/protocol.c | 12 ++++++++----
>> 1 file changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/net/9p/protocol.c b/net/9p/protocol.c
>> index 931ea00c4fed..f1e2425f920b 100644
>> --- a/net/9p/protocol.c
>> +++ b/net/9p/protocol.c
>> @@ -55,16 +55,20 @@ EXPORT_SYMBOL(p9stat_free);
>>
>> size_t pdu_read(struct p9_fcall *pdu, void *data, size_t size)
>> {
>> - size_t len = min(pdu->size - pdu->offset, size);
>> - memcpy(data, &pdu->sdata[pdu->offset], len);
>> + size_t len = pdu->offset > pdu->size ? 0 :
>> + min(pdu->size - pdu->offset, size);
>> + if (len != 0)
>> + memcpy(data, &pdu->sdata[pdu->offset], len);
>> pdu->offset += len;
>> return size - len;
>> }
>>
>> static size_t pdu_write(struct p9_fcall *pdu, const void *data, size_t size)
>> {
>> - size_t len = min(pdu->capacity - pdu->size, size);
>> - memcpy(&pdu->sdata[pdu->size], data, len);
>> + size_t len = pdu->size > pdu->capacity ? 0 :
>> + min(pdu->capacity - pdu->size, size);
>> + if (len != 0)
>> + memcpy(&pdu->sdata[pdu->size], data, len);
>> pdu->size += len;
>> return size - len;
>> }
>>
^ permalink raw reply
* Re: [PATCH bpf 1/1] bpf: btf: Fix bitfield extraction for big endian
From: Daniel Borkmann @ 2018-07-10 8:21 UTC (permalink / raw)
To: Martin KaFai Lau, Okash Khawaja
Cc: Alexei Starovoitov, Yonghong Song, Jakub Kicinski,
David S. Miller, netdev, kernel-team, linux-kernel
In-Reply-To: <20180709183236.r4b7gzmev5h4lcbw@kafai-mbp.dhcp.thefacebook.com>
On 07/09/2018 08:32 PM, Martin KaFai Lau wrote:
> On Sun, Jul 08, 2018 at 05:22:03PM -0700, Okash Khawaja wrote:
>> When extracting bitfield from a number, btf_int_bits_seq_show() builds
>> a mask and accesses least significant byte of the number in a way
>> specific to little-endian. This patch fixes that by checking endianness
>> of the machine and then shifting left and right the unneeded bits.
>>
>> Thanks to Martin Lau for the help in navigating potential pitfalls when
>> dealing with endianess and for the final solution.
>>
>> Fixes: b00b8daec828 ("bpf: btf: Add pretty print capability for data with BTF type info")
>> Signed-off-by: Okash Khawaja <osk@fb.com>
>>
>> ---
>> kernel/bpf/btf.c | 32 +++++++++++++++-----------------
>> 1 file changed, 15 insertions(+), 17 deletions(-)
>>
>> --- a/kernel/bpf/btf.c
>> +++ b/kernel/bpf/btf.c
>> @@ -162,6 +162,8 @@
>> #define BITS_ROUNDDOWN_BYTES(bits) ((bits) >> 3)
>> #define BITS_ROUNDUP_BYTES(bits) \
>> (BITS_ROUNDDOWN_BYTES(bits) + !!BITS_PER_BYTE_MASKED(bits))
>> +const int one = 1;
>> +#define is_big_endian() ((*(char *)&one) == 0)
Also here, in the kernel archs provide proper definitions.
>> #define BTF_INFO_MASK 0x0f00ffff
>> #define BTF_INT_MASK 0x0fffffff
>> @@ -991,16 +993,13 @@ static void btf_int_bits_seq_show(const
>> void *data, u8 bits_offset,
>> struct seq_file *m)
>> {
>> + u8 left_shift_bits, right_shift_bits;
> Nit.
> Although only max 64 bit int is allowed now (ensured by btf_int_check_meta),
> it is better to use u16 such that it will be consistent to BTF_INT_BITS.
>
>> u32 int_data = btf_type_int(t);
>> u16 nr_bits = BTF_INT_BITS(int_data);
>> u16 total_bits_offset;
>> u16 nr_copy_bytes;
>> u16 nr_copy_bits;
>> - u8 nr_upper_bits;
>> - union {
>> - u64 u64_num;
>> - u8 u8_nums[8];
>> - } print_num;
>> + u64 print_num;
>>
>> total_bits_offset = bits_offset + BTF_INT_OFFSET(int_data);
>> data += BITS_ROUNDDOWN_BYTES(total_bits_offset);
>> @@ -1008,21 +1007,20 @@ static void btf_int_bits_seq_show(const
>> nr_copy_bits = nr_bits + bits_offset;
>> nr_copy_bytes = BITS_ROUNDUP_BYTES(nr_copy_bits);
>>
>> - print_num.u64_num = 0;
>> - memcpy(&print_num.u64_num, data, nr_copy_bytes);
>> -
>> - /* Ditch the higher order bits */
>> - nr_upper_bits = BITS_PER_BYTE_MASKED(nr_copy_bits);
>> - if (nr_upper_bits) {
>> - /* We need to mask out some bits of the upper byte. */
>> - u8 mask = (1 << nr_upper_bits) - 1;
>> -
>> - print_num.u8_nums[nr_copy_bytes - 1] &= mask;
>> + print_num = 0;
>> + memcpy(&print_num, data, nr_copy_bytes);
>> + if (is_big_endian()) {
>> + left_shift_bits = bits_offset;
>> + right_shift_bits = BITS_PER_U64 - nr_bits;
>> + } else {
>> + left_shift_bits = BITS_PER_U64 - nr_copy_bits;
>> + right_shift_bits = BITS_PER_U64 - nr_bits;
> Nit.
> right_shift_bits is the same for both cases. Lets simplify it.
>
>> }
>>
>> - print_num.u64_num >>= bits_offset;
>> + print_num <<= left_shift_bits;
>> + print_num >>= right_shift_bits;
>>
>> - seq_printf(m, "0x%llx", print_num.u64_num);
>> + seq_printf(m, "0x%llx", print_num);
>> }
>>
>> static void btf_int_seq_show(const struct btf *btf, const struct btf_type *t,
>>
^ permalink raw reply
* Re: [V9fs-developer] [PATCH] p9_check_errors() validate PDU length
From: Tomas Bortoli @ 2018-07-10 8:15 UTC (permalink / raw)
To: Dominique Martinet
Cc: ericvh, rminnich, lucho, netdev, linux-kernel, syzkaller,
v9fs-developer, davem, Al Viro
In-Reply-To: <20180710022819.GA19285@nautica>
On 07/10/2018 04:28 AM, Dominique Martinet wrote:
> Tomas Bortoli wrote on Tue, Jul 10, 2018:
>> As suggested by Dominique:
>> https://lkml.org/lkml/2018/7/9/688
>> Such check is not enough as it will prevent to read more than how it has
>> been allocated but it won't prevent to read more than how it has been read
>> So this patch will require some more changes to prevent bad sizes.
> Sorry, I'm the one who suggested to put a note after the commit message
> and I didn't see it.
>
> Let's get the proper fix right away, it's not much further.
I agree.
>
>> Also, they really need to check against the actual read size, not just
>> capacity.
>> For virtio/rdma, something like this ought to fix pdu->size, then
>> p9_parse_header can just never overwrite it (untested but it's useless
>> on its own, I'll test the full patch with the parse header change)
> I actually took the time to test a bit; I had only suggested something
> for virtio/rdma because I had assumed trans_fd (the socket transport
> actually used by syzbot) was setting the length in the fcall, but I read
> that code too fast this morning and it is not (it only sets the size in
> its private struct)
>
> Something like that ought to work for trans_fd:
> diff --git a/net/9p/trans_fd.c b/net/9p/trans_fd.c
> index 588bf88c3305..9f3ce370c685 100644
> --- a/net/9p/trans_fd.c
> +++ b/net/9p/trans_fd.c
> @@ -369,6 +370,7 @@ static void p9_read_work(struct work_struct *work)
> */
> if ((m->req) && (m->rc.offset == m->rc.capacity)) {
> p9_debug(P9_DEBUG_TRANS, "got new packet\n");
> + m->req->rc->size = m->rc.offset;
> spin_lock(&m->client->lock);
> if (m->req->status != REQ_STATUS_ERROR)
> status = REQ_STATUS_RCVD;
> ---
>
> This however gets more complicated once you start factoring in that
> change I suggested about p9_parse_header not setting size (and checking
> size) because trans_fd relies on it; so I'm not sure how we should
> proceed.
Mmh, me neither. I don't see where the *actual* PDU length is stored.
>
> Do you have a working 9p tcp server to test changes are valid, or are
> you only working off the syzbot reproducer?
No, I was just using the reproducer to test.
> In the first place, are you willing to take the time to do that bigger
> fix?
Yes.
>
> At this point I can either help you get a working setup and let you do
> the rest, or just finish the bigger patch myself and add you as whatever
> tag you feel comfortable with (persumably Signed-off-by)
>
> Thanks again for starting this,
For me both ways are good. Signed-off-by will be good.
You know for sure more than me about 9p as I started delving it for the
first time yesterday. I can also make and test a patch but I'll need to
understand more about it. Let me know if you find out more.
^ permalink raw reply
* general protection fault in p9_fd_create_unix
From: syzbot @ 2018-07-10 8:09 UTC (permalink / raw)
To: davem, ericvh, linux-kernel, lucho, netdev, rminnich,
syzkaller-bugs, v9fs-developer
Hello,
syzbot found the following crash on:
HEAD commit: 092150a25cb7 Merge branch 'for-linus' of git://git.kernel...
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=15f31a2c400000
kernel config: https://syzkaller.appspot.com/x/.config?x=25856fac4e580aa7
dashboard link: https://syzkaller.appspot.com/bug?extid=1a262da37d3bead15c39
compiler: gcc (GCC) 8.0.1 20180413 (experimental)
userspace arch: i386
syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=17492cc2400000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=145aec58400000
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+1a262da37d3bead15c39@syzkaller.appspotmail.com
random: sshd: uninitialized urandom read (32 bytes read)
random: sshd: uninitialized urandom read (32 bytes read)
random: sshd: uninitialized urandom read (32 bytes read)
kasan: CONFIG_KASAN_INLINE enabled
kasan: GPF could be caused by NULL-ptr deref or user memory access
general protection fault: 0000 [#1] SMP KASAN
CPU: 1 PID: 4527 Comm: syz-executor611 Not tainted 4.18.0-rc4+ #42
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
RIP: 0010:strlen+0x1f/0xa0 lib/string.c:479
Code: ff ff ff 0f 1f 84 00 00 00 00 00 48 b8 00 00 00 00 00 fc ff df 55 48
89 fa 48 c1 ea 03 48 89 e5 41 54 49 89 fc 53 48 83 ec 08 <0f> b6 04 02 48
89 fa 83 e2 07 38 d0 7f 04 84 c0 75 4d 41 80 3c 24
RSP: 0018:ffff8801ad1d7398 EFLAGS: 00010292
RAX: dffffc0000000000 RBX: 1ffff10035a3ae7b RCX: ffffffff8766d02b
RDX: 0000000000000000 RSI: ffffffff87679de2 RDI: 0000000000000000
RBP: ffff8801ad1d73b0 R08: ffff8801d9762200 R09: 0000000000000000
R10: ffffed003b5e46d6 R11: ffff8801daf236b3 R12: 0000000000000000
R13: dffffc0000000000 R14: 0000000000000000 R15: 1ffff10035a3aeb2
FS: 0000000000000000(0000) GS:ffff8801daf00000(0063) knlGS:0000000009aa8840
CS: 0010 DS: 002b ES: 002b CR0: 0000000080050033
CR2: 00000000200003c0 CR3: 00000001d96e0000 CR4: 00000000001406e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
strlen include/linux/string.h:267 [inline]
p9_fd_create_unix+0xbf/0x370 net/9p/trans_fd.c:993
p9_client_create+0x915/0x16c9 net/9p/client.c:1062
v9fs_session_init+0x21a/0x1a80 fs/9p/v9fs.c:400
v9fs_mount+0x7c/0x900 fs/9p/vfs_super.c:135
mount_fs+0xae/0x328 fs/super.c:1277
vfs_kern_mount.part.34+0xdc/0x4e0 fs/namespace.c:1037
vfs_kern_mount fs/namespace.c:1027 [inline]
do_new_mount fs/namespace.c:2518 [inline]
do_mount+0x581/0x30e0 fs/namespace.c:2848
__do_compat_sys_mount fs/compat.c:125 [inline]
__se_compat_sys_mount fs/compat.c:92 [inline]
__ia32_compat_sys_mount+0x5d5/0x860 fs/compat.c:92
do_syscall_32_irqs_on arch/x86/entry/common.c:326 [inline]
do_fast_syscall_32+0x34d/0xfb2 arch/x86/entry/common.c:397
entry_SYSENTER_compat+0x70/0x7f arch/x86/entry/entry_64_compat.S:139
RIP: 0023:0xf7f09cb9
Code: 55 08 8b 88 64 cd ff ff 8b 98 68 cd ff ff 89 c8 85 d2 74 02 89 0a 5b
5d c3 8b 04 24 c3 8b 1c 24 c3 51 52 55 89 e5 0f 34 cd 80 <5d> 5a 59 c3 90
90 90 90 eb 0d 90 90 90 90 90 90 90 90 90 90 90 90
RSP: 002b:00000000ff943bac EFLAGS: 00000286 ORIG_RAX: 0000000000000015
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00000000200000c0
RDX: 0000000020000340 RSI: 0000000000000000 RDI: 00000000200006c0
RBP: 0000000000000008 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
Modules linked in:
Dumping ftrace buffer:
(ftrace buffer empty)
---[ end trace fe903ba4024485fe ]---
RIP: 0010:strlen+0x1f/0xa0 lib/string.c:479
Code: ff ff ff 0f 1f 84 00 00 00 00 00 48 b8 00 00 00 00 00 fc ff df 55 48
89 fa 48 c1 ea 03 48 89 e5 41 54 49 89 fc 53 48 83 ec 08 <0f> b6 04 02 48
89 fa 83 e2 07 38 d0 7f 04 84 c0 75 4d 41 80 3c 24
RSP: 0018:ffff8801ad1d7398 EFLAGS: 00010292
RAX: dffffc0000000000 RBX: 1ffff10035a3ae7b RCX: ffffffff8766d02b
RDX: 0000000000000000 RSI: ffffffff87679de2 RDI: 0000000000000000
RBP: ffff8801ad1d73b0 R08: ffff8801d9762200 R09: 0000000000000000
R10: ffffed003b5e46d6 R11: ffff8801daf236b3 R12: 0000000000000000
R13: dffffc0000000000 R14: 0000000000000000 R15: 1ffff10035a3aeb2
FS: 0000000000000000(0000) GS:ffff8801daf00000(0063) knlGS:0000000009aa8840
CS: 0010 DS: 002b ES: 002b CR0: 0000000080050033
CR2: 00000000200003c0 CR3: 00000001d96e0000 CR4: 00000000001406e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
---
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
* Re: [PATCH] netfilter: NFT_SOCKET don't use NF_SOCKET_IPV6 without NF_TABLES_IPV6
From: Máté Eckl @ 2018-07-10 8:05 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
David S. Miller, Flavio Leitner, netfilter-devel, coreteam,
netdev, linux-kernel
In-Reply-To: <20180710080227.qwh53ahq26j6phhd@sch.bme.hu>
On Tue, Jul 10, 2018 at 10:02:27AM +0200, Máté Eckl wrote:
> On Mon, Jul 09, 2018 at 11:35:09PM +0200, Arnd Bergmann wrote:
> > It is now possible to build the nft_socket module as built-in when
> > NF_TABLES_IPV6 is disabled, and have NF_SOCKET_IPV6=m set manually.
> >
> > In this case, the NF_SOCKET_IPV6 functionality will be useless according
> > to the explanation in commit 35bf1ccecaaa ("netfilter: Kconfig: Change
> > IPv6 select dependencies"), but on top of that it also causes a link
> > error:
> >
> > net/netfilter/nft_socket.o: In function `nft_socket_eval':
> > nft_socket.c:(.text+0x162): undefined reference to `nf_sk_lookup_slow_v6'
> >
> > This changes the compile-time check so we don't attempt to use
> > the NF_SOCKET_IPV6 code when it cannot be used, and make it all
> > compile again. That may lead to unexpected behavior when a user
> > enables NF_SOCKET_IPV6 but cannot use it, but seems to be the
> > logical conclusion of the 35bf1ccecaaa change.
> >
> > Fixes: 35bf1ccecaaa ("netfilter: Kconfig: Change IPv6 select dependencies")
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>
> I think this should be fixed in the Kconfig rather than inside the module(s).
>
> I did some investigation and it turns out that you missed a circumstance. This
> link error occures only if NFT_SOCKET=y && NF_SOCKET_IPV6=m && NF_TABLES_IPV6=y
> (cannot be m here if NFT_SOCKET is y). And probably the same with
> iptables-related modules. Probably this possibility should be eliminated.
NF_TPROXY_IPV6 might be in the same situation.
^ permalink raw reply
* Re: [PATCH bpf] bpf: fix some bad __rcu annotations in bpf/core.c
From: Daniel Borkmann @ 2018-07-10 8:03 UTC (permalink / raw)
To: Roman Gushchin, netdev; +Cc: linux-kernel, kernel-team, Alexei Starovoitov, yhs
In-Reply-To: <20180710015915.26358-1-guro@fb.com>
Hi Roman,
On 07/10/2018 03:59 AM, Roman Gushchin wrote:
> Sparse shows some "incorrect type" warnings in the bpf core code.
Thanks for taking a stab at these! It would really help if you could
split the patch into a small series and fix each individual case that
is problematic here.
Please also add Fixes tags to the patches.
More below.
> They are caused by bad __rcu annotations:
> 1) bpf_prog_array_alloc() returns an __rcu pointer, which isn't true.
> At that moment it's obviously an exclusive "owning" pointer,
> which is valid for an infinite amount of time, so __rcu is
> meaningless.
> 2) The progs local variable in compute_effective_progs should be
> marked as __bpf too, it's a local variable, not shared with anyone
Typo: __bpf ?
> else at all. The real __rcu variable is array pointer, which should
> be assigned with rcu_assign_pointer.
> 3) __rcu progs argument of bpf_prog_array_free() should be casted
> to a simple pointer before calling kfree_rcu().
> 4) There is a missing rcu_dereference() annotation in
> bpf_prog_array_copy_to_user().
> 5) old_array __rcu pointer in bpf_prog_array_copy() is used as
> a "normal" non-__rcu pointer.
>
> These changes remove the following sparse warnings:
> kernel/bpf/core.c:1544:31: warning: incorrect type in return expression (different address spaces)
> kernel/bpf/core.c:1544:31: expected struct bpf_prog_array [noderef] <asn:4>*
> kernel/bpf/core.c:1544:31: got void *
> kernel/bpf/core.c:1548:17: warning: incorrect type in return expression (different address spaces)
> kernel/bpf/core.c:1548:17: expected struct bpf_prog_array [noderef] <asn:4>*
> kernel/bpf/core.c:1548:17: got struct bpf_prog_array *<noident>
> kernel/bpf/core.c:1556:9: warning: incorrect type in argument 1 (different address spaces)
> kernel/bpf/core.c:1556:9: expected struct callback_head *head
> kernel/bpf/core.c:1556:9: got struct callback_head [noderef] <asn:4>*<noident>
> kernel/bpf/core.c:1629:34: warning: incorrect type in initializer (different address spaces)
> kernel/bpf/core.c:1629:34: expected struct bpf_prog **prog
> kernel/bpf/core.c:1629:34: got struct bpf_prog *[noderef] <asn:4>*<noident>
> kernel/bpf/core.c:1653:31: warning: incorrect type in assignment (different address spaces)
> kernel/bpf/core.c:1653:31: expected struct bpf_prog **existing_prog
> kernel/bpf/core.c:1653:31: got struct bpf_prog *[noderef] <asn:4>*<noident>
> kernel/bpf/core.c:1681:15: warning: incorrect type in assignment (different address spaces)
> kernel/bpf/core.c:1681:15: expected struct bpf_prog_array *array
> kernel/bpf/core.c:1681:15: got struct bpf_prog_array [noderef] <asn:4>*
> kernel/bpf/core.c:1687:31: warning: incorrect type in assignment (different address spaces)
> kernel/bpf/core.c:1687:31: expected struct bpf_prog **[assigned] existing_prog
> kernel/bpf/core.c:1687:31: got struct bpf_prog *[noderef] <asn:4>*<noident>
>
> Signed-off-by: Roman Gushchin <guro@fb.com>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Cc: Alexei Starovoitov <ast@kernel.org>
> ---
> include/linux/bpf.h | 2 +-
> kernel/bpf/cgroup.c | 7 +++----
> kernel/bpf/core.c | 14 ++++++++------
> 3 files changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 8827e797ff97..943fb08d8287 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -352,7 +352,7 @@ struct bpf_prog_array {
> struct bpf_prog *progs[0];
> };
>
> -struct bpf_prog_array __rcu *bpf_prog_array_alloc(u32 prog_cnt, gfp_t flags);
> +struct bpf_prog_array *bpf_prog_array_alloc(u32 prog_cnt, gfp_t flags);
> void bpf_prog_array_free(struct bpf_prog_array __rcu *progs);
> int bpf_prog_array_length(struct bpf_prog_array __rcu *progs);
> int bpf_prog_array_copy_to_user(struct bpf_prog_array __rcu *progs,
> diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
> index 3d83ee7df381..badabb0b435c 100644
> --- a/kernel/bpf/cgroup.c
> +++ b/kernel/bpf/cgroup.c
> @@ -95,7 +95,7 @@ static int compute_effective_progs(struct cgroup *cgrp,
> enum bpf_attach_type type,
> struct bpf_prog_array __rcu **array)
> {
> - struct bpf_prog_array __rcu *progs;
> + struct bpf_prog_array *progs;
> struct bpf_prog_list *pl;
> struct cgroup *p = cgrp;
> int cnt = 0;
> @@ -120,13 +120,12 @@ static int compute_effective_progs(struct cgroup *cgrp,
> &p->bpf.progs[type], node) {
> if (!pl->prog)
> continue;
> - rcu_dereference_protected(progs, 1)->
> - progs[cnt++] = pl->prog;
> + progs->progs[cnt++] = pl->prog;
> }
> p = cgroup_parent(p);
> } while (p);
>
> - *array = progs;
> + rcu_assign_pointer(*array, progs);
> return 0;
> }
>
> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> index 1e5625d46414..f6e5b207a0d7 100644
> --- a/kernel/bpf/core.c
> +++ b/kernel/bpf/core.c
> @@ -1538,7 +1538,7 @@ static struct {
> .null_prog = NULL,
> };
>
> -struct bpf_prog_array __rcu *bpf_prog_array_alloc(u32 prog_cnt, gfp_t flags)
> +struct bpf_prog_array *bpf_prog_array_alloc(u32 prog_cnt, gfp_t flags)
> {
> if (prog_cnt)
> return kzalloc(sizeof(struct bpf_prog_array) +
> @@ -1550,10 +1550,11 @@ struct bpf_prog_array __rcu *bpf_prog_array_alloc(u32 prog_cnt, gfp_t flags)
Looks good to me to here.
> void bpf_prog_array_free(struct bpf_prog_array __rcu *progs)
> {
> - if (!progs ||
> - progs == (struct bpf_prog_array __rcu *)&empty_prog_array.hdr)
> + struct bpf_prog_array *array = rcu_access_pointer(progs);
Can you elaborate on the rcu_access_pointer() part? This looks odd, at minimum
this needs a comment explaining why it's needed. Is the __rcu annotation above
even correct?
> +
> + if (!array || array == &empty_prog_array.hdr)
> return;
> - kfree_rcu(progs, rcu);
> + kfree_rcu(array, rcu);
> }
>
> int bpf_prog_array_length(struct bpf_prog_array __rcu *progs)
> @@ -1626,7 +1627,7 @@ int bpf_prog_array_copy_to_user(struct bpf_prog_array __rcu *progs,
> void bpf_prog_array_delete_safe(struct bpf_prog_array __rcu *progs,
> struct bpf_prog *old_prog)
> {
> - struct bpf_prog **prog = progs->progs;
> + struct bpf_prog **prog = rcu_dereference(progs)->progs;
Can you elaborate here as well? __rcu annotation buggy instead?
> for (; *prog; prog++)
> if (*prog == old_prog) {
> @@ -1635,11 +1636,12 @@ void bpf_prog_array_delete_safe(struct bpf_prog_array __rcu *progs,
> }
> }
>
> -int bpf_prog_array_copy(struct bpf_prog_array __rcu *old_array,
> +int bpf_prog_array_copy(struct bpf_prog_array __rcu *__old_array,
> struct bpf_prog *exclude_prog,
> struct bpf_prog *include_prog,
> struct bpf_prog_array **new_array)
> {
> + struct bpf_prog_array *old_array = rcu_access_pointer(__old_array);
Same comment here, this doesn't look right. We even fetch old_array->progs
from it later on in this path.
> int new_prog_cnt, carry_prog_cnt = 0;
> struct bpf_prog **existing_prog;
> struct bpf_prog_array *array;
>
Thanks,
Daniel
^ permalink raw reply
* Re: [PATCH] netfilter: NFT_SOCKET don't use NF_SOCKET_IPV6 without NF_TABLES_IPV6
From: Máté Eckl @ 2018-07-10 8:02 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
David S. Miller, Flavio Leitner, netfilter-devel, coreteam,
netdev, linux-kernel
In-Reply-To: <20180709213537.2748896-1-arnd@arndb.de>
On Mon, Jul 09, 2018 at 11:35:09PM +0200, Arnd Bergmann wrote:
> It is now possible to build the nft_socket module as built-in when
> NF_TABLES_IPV6 is disabled, and have NF_SOCKET_IPV6=m set manually.
>
> In this case, the NF_SOCKET_IPV6 functionality will be useless according
> to the explanation in commit 35bf1ccecaaa ("netfilter: Kconfig: Change
> IPv6 select dependencies"), but on top of that it also causes a link
> error:
>
> net/netfilter/nft_socket.o: In function `nft_socket_eval':
> nft_socket.c:(.text+0x162): undefined reference to `nf_sk_lookup_slow_v6'
>
> This changes the compile-time check so we don't attempt to use
> the NF_SOCKET_IPV6 code when it cannot be used, and make it all
> compile again. That may lead to unexpected behavior when a user
> enables NF_SOCKET_IPV6 but cannot use it, but seems to be the
> logical conclusion of the 35bf1ccecaaa change.
>
> Fixes: 35bf1ccecaaa ("netfilter: Kconfig: Change IPv6 select dependencies")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
I think this should be fixed in the Kconfig rather than inside the module(s).
I did some investigation and it turns out that you missed a circumstance. This
link error occures only if NFT_SOCKET=y && NF_SOCKET_IPV6=m && NF_TABLES_IPV6=y
(cannot be m here if NFT_SOCKET is y). And probably the same with
iptables-related modules. Probably this possibility should be eliminated.
^ permalink raw reply
* Re: [PATCH v2] samples/bpf: Fix tc and ip paths in xdp2skb_meta.sh
From: Daniel Borkmann @ 2018-07-10 7:20 UTC (permalink / raw)
To: Taeung Song, Alexei Starovoitov
Cc: Jesper Dangaard Brouer, netdev, linux-kernel
In-Reply-To: <20180709175121.3355-1-treeze.taeung@gmail.com>
On 07/09/2018 07:51 PM, Taeung Song wrote:
> The below path error can occur:
>
> # ./xdp2skb_meta.sh --dev eth0 --list
> ./xdp2skb_meta.sh: line 61: /usr/sbin/tc: No such file or directory
>
> So just use command names instead of absolute paths of tc and ip.
> In addition, it allow callers to redefine $TC and $IP paths
>
> Fixes: 36e04a2d78d9 ("samples/bpf: xdp2skb_meta shows transferring info from XDP to SKB")
> Reviewed-by: Jesper Dangaard Brouer <brouer@redhat.com>
> Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
Applied to bpf, thanks Taeung!
^ permalink raw reply
* [PATCH net-next 3/3] mlxsw: spectrum_span: Change LAG lower selection
From: Ido Schimmel @ 2018-07-10 7:02 UTC (permalink / raw)
To: netdev; +Cc: davem, petrm, jiri, mlxsw, Ido Schimmel
In-Reply-To: <20180710070259.24753-1-idosch@mellanox.com>
From: Petr Machata <petrm@mellanox.com>
When offloading mirror-to-gretap, mlxsw needs to preroute the path that
the encapsulated packet will take. That path may include a LAG device
above a front panel port. So far, mlxsw resolved the path to the first
up front panel slave of the LAG interface, but that only reflects
administrative state of the port. It neglects to consider whether the
port actually has a carrier, and what the LACP state is.
So instead of checking upness of the device, check carrier state and
txability.
Signed-off-by: Petr Machata <petrm@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c
index 3d187d88cc7c..e42d640cddab 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c
@@ -36,6 +36,7 @@
#include <linux/list.h>
#include <net/arp.h>
#include <net/gre.h>
+#include <net/lag.h>
#include <net/ndisc.h>
#include <net/ip6_tunnel.h>
@@ -254,7 +255,9 @@ mlxsw_sp_span_entry_lag(struct net_device *lag_dev)
struct list_head *iter;
netdev_for_each_lower_dev(lag_dev, dev, iter)
- if ((dev->flags & IFF_UP) && mlxsw_sp_port_dev_check(dev))
+ if (netif_carrier_ok(dev) &&
+ net_lag_port_dev_txable(dev) &&
+ mlxsw_sp_port_dev_check(dev))
return dev;
return NULL;
--
2.14.4
^ permalink raw reply related
* [PATCH net-next 2/3] net: Add lag.h, net_lag_port_dev_txable()
From: Ido Schimmel @ 2018-07-10 7:02 UTC (permalink / raw)
To: netdev; +Cc: davem, petrm, jiri, mlxsw, Ido Schimmel
In-Reply-To: <20180710070259.24753-1-idosch@mellanox.com>
From: Petr Machata <petrm@mellanox.com>
LAG devices (team or bond) recognize for each one of their slave devices
whether LAG traffic is going to be sent through that device. Bond calls
such devices "active", team calls them "txable". When this state
changes, a NETDEV_CHANGELOWERSTATE notification is distributed, together
with a netdev_notifier_changelowerstate_info structure that for LAG
devices includes a tx_enabled flag that refers to the new state. The
notification thus makes it possible to react to the changes in txability
in drivers.
However there's no way to query txability from the outside on demand.
That is problematic namely for mlxsw, which when resolving ERSPAN packet
path, may encounter a LAG device, and needs to determine which of the
slaves it should choose.
To that end, introduce a new function, net_lag_port_dev_txable(), which
determines whether a given slave device is "active" or
"txable" (depending on the flavor of the LAG device). That function then
dispatches to per-LAG-flavor helpers, bond_is_active_slave_dev() resp.
team_port_dev_txable().
Because there currently is no good place where net_lag_port_dev_txable()
should be added, introduce a new header file, lag.h, which should from
now on hold any logic common to both team and bond. (But keep
netif_is_lag_master() together with the rest of netif_is_*_master()
functions).
Signed-off-by: Petr Machata <petrm@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
include/linux/if_team.h | 13 +++++++++++++
include/net/bonding.h | 13 +++++++++++++
include/net/lag.h | 17 +++++++++++++++++
3 files changed, 43 insertions(+)
create mode 100644 include/net/lag.h
diff --git a/include/linux/if_team.h b/include/linux/if_team.h
index 0d07c6655cce..ac42da56f7a2 100644
--- a/include/linux/if_team.h
+++ b/include/linux/if_team.h
@@ -89,6 +89,19 @@ static inline bool team_port_txable(struct team_port *port)
return port->linkup && team_port_enabled(port);
}
+static inline bool team_port_dev_txable(const struct net_device *port_dev)
+{
+ struct team_port *port;
+ bool txable;
+
+ rcu_read_lock();
+ port = team_port_get_rcu(port_dev);
+ txable = port ? team_port_txable(port) : false;
+ rcu_read_unlock();
+
+ return txable;
+}
+
#ifdef CONFIG_NET_POLL_CONTROLLER
static inline void team_netpoll_send_skb(struct team_port *port,
struct sk_buff *skb)
diff --git a/include/net/bonding.h b/include/net/bonding.h
index 808f1d167349..a2d058170ea3 100644
--- a/include/net/bonding.h
+++ b/include/net/bonding.h
@@ -411,6 +411,19 @@ static inline bool bond_slave_can_tx(struct slave *slave)
bond_is_active_slave(slave);
}
+static inline bool bond_is_active_slave_dev(const struct net_device *slave_dev)
+{
+ struct slave *slave;
+ bool active;
+
+ rcu_read_lock();
+ slave = bond_slave_get_rcu(slave_dev);
+ active = bond_is_active_slave(slave);
+ rcu_read_unlock();
+
+ return active;
+}
+
static inline void bond_hw_addr_copy(u8 *dst, const u8 *src, unsigned int len)
{
if (len == ETH_ALEN) {
diff --git a/include/net/lag.h b/include/net/lag.h
new file mode 100644
index 000000000000..95b880e6fdde
--- /dev/null
+++ b/include/net/lag.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_IF_LAG_H
+#define _LINUX_IF_LAG_H
+
+#include <linux/netdevice.h>
+#include <linux/if_team.h>
+#include <net/bonding.h>
+
+static inline bool net_lag_port_dev_txable(const struct net_device *port_dev)
+{
+ if (netif_is_team_port(port_dev))
+ return team_port_dev_txable(port_dev);
+ else
+ return bond_is_active_slave_dev(port_dev);
+}
+
+#endif /* _LINUX_IF_LAG_H */
--
2.14.4
^ permalink raw reply related
* [PATCH net-next 1/3] team: Publish team_port_get_rcu()
From: Ido Schimmel @ 2018-07-10 7:02 UTC (permalink / raw)
To: netdev; +Cc: davem, petrm, jiri, mlxsw, Ido Schimmel
In-Reply-To: <20180710070259.24753-1-idosch@mellanox.com>
From: Petr Machata <petrm@mellanox.com>
A follow-up patch adds a new entry point, team_port_dev_txable(). Making
it an ordinary exported function would mean that any module that may
need the service in one of the supported configurations also
unconditionally needs to pull in the team module, whether or not the
user actually intends to create team interfaces.
To prevent that, team_port_dev_txable() is defined in if_team.h, and
therefore all dependencies of that function also need to be
publicly-visible.
Therefore move team_port_get_rcu() from team.c to if_team.h.
Signed-off-by: Petr Machata <petrm@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
drivers/net/team/team.c | 5 -----
include/linux/if_team.h | 5 +++++
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
index b070959737ff..c46191183fb1 100644
--- a/drivers/net/team/team.c
+++ b/drivers/net/team/team.c
@@ -41,11 +41,6 @@
#define team_port_exists(dev) (dev->priv_flags & IFF_TEAM_PORT)
-static struct team_port *team_port_get_rcu(const struct net_device *dev)
-{
- return rcu_dereference(dev->rx_handler_data);
-}
-
static struct team_port *team_port_get_rtnl(const struct net_device *dev)
{
struct team_port *port = rtnl_dereference(dev->rx_handler_data);
diff --git a/include/linux/if_team.h b/include/linux/if_team.h
index d95cae09dea0..0d07c6655cce 100644
--- a/include/linux/if_team.h
+++ b/include/linux/if_team.h
@@ -74,6 +74,11 @@ struct team_port {
long mode_priv[0];
};
+static inline struct team_port *team_port_get_rcu(const struct net_device *dev)
+{
+ return rcu_dereference(dev->rx_handler_data);
+}
+
static inline bool team_port_enabled(struct team_port *port)
{
return port->index != -1;
--
2.14.4
^ permalink raw reply related
* [PATCH net-next 0/3] mlxsw: ERSPAN: Take LACP state into consideration
From: Ido Schimmel @ 2018-07-10 7:02 UTC (permalink / raw)
To: netdev; +Cc: davem, petrm, jiri, mlxsw, Ido Schimmel
Petr says:
When offloading mirror-to-gretap, mlxsw needs to preroute the path that
the encapsulated packet will take. That path may include a LAG device
above a front panel port. So far, mlxsw resolved the path to the first
up front panel slave of the LAG interface, but that only reflects
administrative state of the port. It neglects to consider whether the
port actually has a carrier, and what the LACP state is. This patch set
aims to address these problems.
Patch #1 publishes team_port_get_rcu().
Then in patch #2, a new function is introduced,
mlxsw_sp_port_dev_check(). That returns, for a given netdevice that is a
slave of a LAG device, whether that device is "txable", i.e. whether the
LAG master would send traffic through it. Since there's no good place to
put LAG-wide helpers, introduce a new header include/net/lag.h.
Finally in patch #3, fix the slave selection logic to take into
consideration whether a given slave has a carrier and whether it is
txable.
Petr Machata (3):
team: Publish team_port_get_rcu()
net: Add lag.h, net_lag_port_dev_txable()
mlxsw: spectrum_span: Change LAG lower selection
drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c | 5 ++++-
drivers/net/team/team.c | 5 -----
include/linux/if_team.h | 18 ++++++++++++++++++
include/net/bonding.h | 13 +++++++++++++
include/net/lag.h | 17 +++++++++++++++++
5 files changed, 52 insertions(+), 6 deletions(-)
create mode 100644 include/net/lag.h
--
2.14.4
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox