Netdev List
 help / color / mirror / Atom feed
* Re: [patch] netlink.7: srcfix Change buffer size in example code about reading netlink message.
From: Michael Kerrisk (man-pages) @ 2017-08-16  0:37 UTC (permalink / raw)
  To: dwilder
  Cc: mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w,
	linux-man-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <e9409957290af5249750afa0f10de3a6-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>

On 11/14/2016 11:20 PM, dwilder wrote:
> The example code in netlink(7) (for reading netlink message) suggests 
> using
> a 4k read buffer with recvmsg.  This can cause truncated messages on 
> systems
> using a page size is >4096.  Please see:
> linux/include/linux/netlink.h (in the kernel source)
> 
> <snip>
> /*
>   *      skb should fit one page. This choice is good for headerless 
> malloc.
>   *      But we should limit to 8K so that userspace does not have to
>   *      use enormous buffer sizes on recvmsg() calls just to avoid
>   *      MSG_TRUNC when PAGE_SIZE is very large.
>   */
> #if PAGE_SIZE < 8192UL
> #define NLMSG_GOODSIZE  SKB_WITH_OVERHEAD(PAGE_SIZE)
> #else
> #define NLMSG_GOODSIZE  SKB_WITH_OVERHEAD(8192UL)
> #endif
> 
> #define NLMSG_DEFAULT_SIZE (NLMSG_GOODSIZE - NLMSG_HDRLEN)
> <snip>
> 
> I was troubleshooting some up-stream code on a ppc64le system
> (page:size of 64k) This code had duplicated the example from netlink(7) 
> and
> was using a 4k buffer.  On x86-64 with a 4k page size this is not a 
> problem,
> however on the 64k page system some messages were truncated.  Using an 
> 8k buffer
> as implied in netlink.h prevents problems with any page size.
> 
> Lets change the example so others don't propagate the problem further.
> 
> Signed-off-by David Wilder <dwilder-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

Thanks, David. Patch applied.

Cheers,

Michael


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH net] bpf: fix bpf_trace_printk on 32 bit archs
From: David Miller @ 2017-08-16  0:32 UTC (permalink / raw)
  To: daniel; +Cc: james.hogan, ast, netdev
In-Reply-To: <dd122e1ec0eadaf3f8fc11107f7a57177dcd8588.1502840567.git.daniel@iogearbox.net>

From: Daniel Borkmann <daniel@iogearbox.net>
Date: Wed, 16 Aug 2017 01:45:33 +0200

> James reported that on MIPS32 bpf_trace_printk() is currently
> broken while MIPS64 works fine:
> 
>   bpf_trace_printk() uses conditional operators to attempt to
>   pass different types to __trace_printk() depending on the
>   format operators. This doesn't work as intended on 32-bit
>   architectures where u32 and long are passed differently to
>   u64, since the result of C conditional operators follows the
>   "usual arithmetic conversions" rules, such that the values
>   passed to __trace_printk() will always be u64 [causing issues
>   later in the va_list handling for vscnprintf()].
> 
>   For example the samples/bpf/tracex5 test printed lines like
>   below on MIPS32, where the fd and buf have come from the u64
>   fd argument, and the size from the buf argument:
> 
>     [...] 1180.941542: 0x00000001: write(fd=1, buf=  (null), size=6258688)
> 
>   Instead of this:
> 
>     [...] 1625.616026: 0x00000001: write(fd=1, buf=009e4000, size=512)
> 
> One way to get it working is to expand various combinations
> of argument types into 8 different combinations for 32 bit
> and 64 bit kernels. Fix tested by James on MIPS32 and MIPS64
> as well that it resolves the issue.
> 
> Fixes: 9c959c863f82 ("tracing: Allow BPF programs to call bpf_trace_printk()")
> Reported-by: James Hogan <james.hogan@imgtec.com>
> Tested-by: James Hogan <james.hogan@imgtec.com>
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>

Applied and queued up for -stable, thanks Daniel.

^ permalink raw reply

* Re: [PATCH net RESEND] PCI: fix oops when try to find Root Port for a PCI device
From: David Miller @ 2017-08-16  0:26 UTC (permalink / raw)
  To: helgaas
  Cc: dingtianhong, leedom, ashok.raj, bhelgaas, werner, ganeshgr,
	asit.k.mallick, patrick.j.cramer, Suravee.Suthikulpanit, Bob.Shaw,
	l.stach, amira, gabriele.paoloni, David.Laight, jeffrey.t.kirsher,
	catalin.marinas, will.deacon, mark.rutland, robin.murphy,
	alexander.duyck, eric.dumazet, linux-arm-kernel, netdev,
	linux-pci, linux-kernel, linuxarm
In-Reply-To: <20170815170331.GA4099@bhelgaas-glaptop.roam.corp.google.com>

From: Bjorn Helgaas <helgaas@kernel.org>
Date: Tue, 15 Aug 2017 12:03:31 -0500

> On Tue, Aug 15, 2017 at 11:24:48PM +0800, Ding Tianhong wrote:
>> Eric report a oops when booting the system after applying
>> the commit a99b646afa8a ("PCI: Disable PCIe Relaxed..."):
>> ...
> 
>> It looks like the pci_find_pcie_root_port() was trying to
>> find the Root Port for the PCI device which is the Root
>> Port already, it will return NULL and trigger the problem,
>> so check the highest_pcie_bridge to fix thie problem.
> 
> The problem was actually with a Root Complex Integrated Endpoint that
> has no upstream PCIe device:
> 
>   00:05.2 System peripheral: Intel Corporation Device 0e2a (rev 04)
>         Subsystem: Intel Corporation Device 0e2a
>         Control: I/O- Mem- BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
>         Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
>         Capabilities: [40] Express (v2) Root Complex Integrated Endpoint, MSI 00
>                 DevCap: MaxPayload 128 bytes, PhantFunc 0, Latency L0s <64ns, L1 <1us
>                         ExtTag- RBE- FLReset-
>                 DevCtl: Report errors: Correctable- Non-Fatal- Fatal+ Unsupported+
>                         RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop+
>                         MaxPayload 128 bytes, MaxReadReq 128 bytes
> 
>> Fixes: a99b646afa8a ("PCI: Disable PCIe Relaxed Ordering if unsupported")
> 
> This also
> 
> Fixes: c56d4450eb68 ("PCI: Turn off Request Attributes to avoid Chelsio T5 Completion erratum")
> 
> which added pci_find_pcie_root_port().  Prior to this Relaxed Ordering
> series, we only used pci_find_pcie_root_port() in a Chelsio quirk that
> only applied to non-integrated endpoints, so we didn't trip over the
> bug.
 ...
> I think structuring the fix as follows is a little more readable:
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index af0cc3456dc1..587cd7623ed8 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c

I've integrated all of this feedback and the other Fixes: tag and applied it
to 'net', thanks.

^ permalink raw reply

* Re: [PATCH ] dt-bindings: net: ravb : Add support for r8a7745 SoC
From: David Miller @ 2017-08-16  0:23 UTC (permalink / raw)
  To: biju.das
  Cc: robh+dt, mark.rutland, linux, sergei.shtylyov, horms, magnus.damm,
	chris.paterson2, devicetree, linux-renesas-soc, linux-arm-kernel,
	netdev
In-Reply-To: <1502808020-22406-1-git-send-email-biju.das@bp.renesas.com>

From: Biju Das <biju.das@bp.renesas.com>
Date: Tue, 15 Aug 2017 15:40:20 +0100

> Add a new compatible string for the RZ/G1E (R8A7745) SoC.
> 
> Signed-off-by: Biju Das <biju.das@bp.renesas.com>
> ---
> This patch is tested against linux-next tag next-20170815
> as well as net-next.

Applied.

^ permalink raw reply

* Re: [PATCH net-next 0/4] inet: make RTM_GETROUTE work without rtnl
From: David Miller @ 2017-08-16  0:21 UTC (permalink / raw)
  To: fw; +Cc: netdev
In-Reply-To: <20170815143444.17002-1-fw@strlen.de>

From: Florian Westphal <fw@strlen.de>
Date: Tue, 15 Aug 2017 16:34:40 +0200

> ipv4 getroute doesn't assume rtnl lock is held anymore, also make
> this true for ipv6, then switch both to DOIT_UNLOCKED.

Series applied, thanks Florian.

^ permalink raw reply

* https://www.spinics.net/lists/kernel/ non work
From: Алексей Болдырев @ 2017-08-16  0:20 UTC (permalink / raw)
  To: netdev

Forbidden

You don't have permission to access /lists/kernel/ on this server.
Apache/2.4.6 (CentOS) Server at www.spinics.net Port 443

^ permalink raw reply

* Re: [PATCH net] sfc: don't try and read ef10 data on non-ef10 NIC
From: David Miller @ 2017-08-16  0:19 UTC (permalink / raw)
  To: bkenward; +Cc: netdev, linux-net-drivers, sbrivio
In-Reply-To: <45a53c3b-b578-56ed-ef41-09bef5b5519c@solarflare.com>

From: Bert Kenward <bkenward@solarflare.com>
Date: Tue, 15 Aug 2017 14:55:32 +0100

> The MAC stats command takes a port ID, which doesn't exist on
> pre-ef10 NICs (5000- and 6000- series). This is extracted from the
> NIC specific data; we misinterpret this as the ef10 data structure,
> causing us to read potentially unallocated data. With a KASAN kernel
> this can cause errors with:
>    BUG: KASAN: slab-out-of-bounds in efx_mcdi_mac_stats
> 
> Fixes: 0a2ab4d988d7 ("sfc: set the port-id when calling MC_CMD_MAC_STATS")
> Reported-by: Stefano Brivio <sbrivio@redhat.com>
> Tested-by: Stefano Brivio <sbrivio@redhat.com>
> Signed-off-by: Bert Kenward <bkenward@solarflare.com>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH net-next] dsa: fix flow disector null pointer
From: David Miller @ 2017-08-16  0:19 UTC (permalink / raw)
  To: kraigatgoog; +Cc: john, andrew, dvyukov, netdev
In-Reply-To: <20170815134340.169914-1-kraigatgoog@gmail.com>

From: Craig Gallek <kraigatgoog@gmail.com>
Date: Tue, 15 Aug 2017 09:43:40 -0400

> From: Craig Gallek <kraig@google.com>
> 
> A recent change to fix up DSA device behavior made the assumption that
> all skbs passing through the flow disector will be associated with a
> device. This does not appear to be a safe assumption.  Syzkaller found
> the crash below by attaching a BPF socket filter that tries to find the
> payload offset of a packet passing between two unix sockets.
 ...
> Fixes: 43e665287f93 ("net-next: dsa: fix flow dissection")
> Reported-by: Dmitry Vyukov <dvyukov@google.com>
> Signed-off-by: Craig Gallek <kraig@google.com>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH] net_sched: remove warning from qdisc_hash_add
From: David Miller @ 2017-08-16  0:17 UTC (permalink / raw)
  To: khlebnikov; +Cc: netdev, xiyou.wangcong, jkosina, edumazet, jhs
In-Reply-To: <150280434474.717808.1684163881218427525.stgit@buzz>

From: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
Date: Tue, 15 Aug 2017 16:39:05 +0300

> It was added in commit e57a784d8cae ("pkt_sched: set root qdisc
> before change() in attach_default_qdiscs()") to hide duplicates
> from "tc qdisc show" for incative deivices.
> 
> After 59cc1f61f ("net: sched: convert qdisc linked list to hashtable")
> it triggered when classful qdisc is added to inactive device because
> default qdiscs are added before switching root qdisc.
> 
> Anyway after commit ea3274695353 ("net: sched: avoid duplicates in
> qdisc dump") duplicates are filtered right in dumper.
> 
> Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>

Applied.

^ permalink raw reply

* Re: [PATCH] net_sched/sfq: update hierarchical backlog when drop packet
From: David Miller @ 2017-08-16  0:17 UTC (permalink / raw)
  To: khlebnikov; +Cc: netdev, xiyou.wangcong, jhs
In-Reply-To: <150280422406.717624.15125218636935056461.stgit@buzz>

From: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
Date: Tue, 15 Aug 2017 16:37:04 +0300

> When sfq_enqueue() drops head packet or packet from another queue it
> have to update backlog at upper qdiscs too.
> 
> Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
> Fixes: 2ccccf5fb43f ("net_sched: update hierarchical backlog too")

Applied and queued up for -stable.

^ permalink raw reply

* Re: [PATCH] net_sched: reset pointers to tcf blocks in classful qdiscs' destructors
From: David Miller @ 2017-08-16  0:16 UTC (permalink / raw)
  To: khlebnikov; +Cc: netdev, xiyou.wangcong, jiri, jhs
In-Reply-To: <150280412126.717517.16274927544808177918.stgit@buzz>

From: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
Date: Tue, 15 Aug 2017 16:35:21 +0300

> Traffic filters could keep direct pointers to classes in classful qdisc,
> thus qdisc destruction first removes all filters before freeing classes.
> Class destruction methods also tries to free attached filters but now
> this isn't safe because tcf_block_put() unlike to tcf_destroy_chain()
> cannot be called second time.
> 
> This patch set class->block to NULL after first tcf_block_put() and
> turn second call into no-op.
> 
> Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
> Fixes: 6529eaba33f0 ("net: sched: introduce tcf block infractructure")

Applied and queued up for -stable.

^ permalink raw reply

* Re: [PATCH V4 net 0/2] ipv6: fix flowlabel issue for reset packet
From: Tom Herbert @ 2017-08-16  0:15 UTC (permalink / raw)
  To: Shaohua Li
  Cc: Linux Kernel Network Developers, David S. Miller,
	Martin KaFai Lau
In-Reply-To: <20170815224252.6vjkyqsngybl3z3m@kernel.org>

On Tue, Aug 15, 2017 at 3:42 PM, Shaohua Li <shli@kernel.org> wrote:
> On Tue, Aug 15, 2017 at 07:08:31AM -0700, Tom Herbert wrote:
>> On Mon, Aug 14, 2017 at 7:52 PM, Shaohua Li <shli@kernel.org> wrote:
>> > On Fri, Aug 11, 2017 at 06:00:20PM -0700, Tom Herbert wrote:
>> >> On Thu, Aug 10, 2017 at 12:13 PM, Shaohua Li <shli@kernel.org> wrote:
>> >> > On Thu, Aug 10, 2017 at 11:30:51AM -0700, Tom Herbert wrote:
>> >> >> On Thu, Aug 10, 2017 at 9:30 AM, Shaohua Li <shli@kernel.org> wrote:
>> >> >> > On Wed, Aug 09, 2017 at 09:40:08AM -0700, Tom Herbert wrote:
>> >> >> >> On Mon, Jul 31, 2017 at 3:19 PM, Shaohua Li <shli@kernel.org> wrote:
>> >> >> >> > From: Shaohua Li <shli@fb.com>
>> >> >> >> >
>> >> >> >> > Please see below tcpdump output:
>> >> >> >> > 21:00:48.109122 IP6 (flowlabel 0x43304, hlim 64, next-header TCP (6) payload length: 40) fec0::5054:ff:fe12:3456.55804 > fec0::5054:ff:fe12:3456.5555: Flags [S], cksum 0x0529 (incorrect -> 0xf56c), seq 3282214508, win 43690, options [mss 65476,sackOK,TS val 2500903437 ecr 0,nop,wscale 7], length 0
>> >> >> >> > 21:00:48.109381 IP6 (flowlabel 0xd827f, hlim 64, next-header TCP (6) payload length: 40) fec0::5054:ff:fe12:3456.5555 > fec0::5054:ff:fe12:3456.55804: Flags [S.], cksum 0x0529 (incorrect -> 0x49ad), seq 1923801573, ack 3282214509, win 43690, options [mss 65476,sackOK,TS val 2500903437 ecr 2500903437,nop,wscale 7], length 0
>> >> >> >> > 21:00:48.109548 IP6 (flowlabel 0x43304, hlim 64, next-header TCP (6) payload length: 32) fec0::5054:ff:fe12:3456.55804 > fec0::5054:ff:fe12:3456.5555: Flags [.], cksum 0x0521 (incorrect -> 0x1bdf), seq 1, ack 1, win 342, options [nop,nop,TS val 2500903437 ecr 2500903437], length 0
>> >> >> >> > 21:00:48.109823 IP6 (flowlabel 0x43304, hlim 64, next-header TCP (6) payload length: 62) fec0::5054:ff:fe12:3456.55804 > fec0::5054:ff:fe12:3456.5555: Flags [P.], cksum 0x053f (incorrect -> 0xb8b1), seq 1:31, ack 1, win 342, options [nop,nop,TS val 2500903437 ecr 2500903437], length 30
>> >> >> >> > 21:00:48.109910 IP6 (flowlabel 0xd827f, hlim 64, next-header TCP (6) payload length: 32) fec0::5054:ff:fe12:3456.5555 > fec0::5054:ff:fe12:3456.55804: Flags [.], cksum 0x0521 (incorrect -> 0x1bc1), seq 1, ack 31, win 342, options [nop,nop,TS val 2500903437 ecr 2500903437], length 0
>> >> >> >> > 21:00:48.110043 IP6 (flowlabel 0xd827f, hlim 64, next-header TCP (6) payload length: 56) fec0::5054:ff:fe12:3456.5555 > fec0::5054:ff:fe12:3456.55804: Flags [P.], cksum 0x0539 (incorrect -> 0xb726), seq 1:25, ack 31, win 342, options [nop,nop,TS val 2500903438 ecr 2500903437], length 24
>> >> >> >> > 21:00:48.110173 IP6 (flowlabel 0x43304, hlim 64, next-header TCP (6) payload length: 32) fec0::5054:ff:fe12:3456.55804 > fec0::5054:ff:fe12:3456.5555: Flags [.], cksum 0x0521 (incorrect -> 0x1ba7), seq 31, ack 25, win 342, options [nop,nop,TS val 2500903438 ecr 2500903438], length 0
>> >> >> >> > 21:00:48.110211 IP6 (flowlabel 0xd827f, hlim 64, next-header TCP (6) payload length: 32) fec0::5054:ff:fe12:3456.5555 > fec0::5054:ff:fe12:3456.55804: Flags [F.], cksum 0x0521 (incorrect -> 0x1ba7), seq 25, ack 31, win 342, options [nop,nop,TS val 2500903438 ecr 2500903437], length 0
>> >> >> >> > 21:00:48.151099 IP6 (flowlabel 0x43304, hlim 64, next-header TCP (6) payload length: 32) fec0::5054:ff:fe12:3456.55804 > fec0::5054:ff:fe12:3456.5555: Flags [.], cksum 0x0521 (incorrect -> 0x1ba6), seq 31, ack 26, win 342, options [nop,nop,TS val 2500903438 ecr 2500903438], length 0
>> >> >> >> > 21:00:49.110524 IP6 (flowlabel 0x43304, hlim 64, next-header TCP (6) payload length: 56) fec0::5054:ff:fe12:3456.55804 > fec0::5054:ff:fe12:3456.5555: Flags [P.], cksum 0x0539 (incorrect -> 0xb324), seq 31:55, ack 26, win 342, options [nop,nop,TS val 2500904438 ecr 2500903438], length 24
>> >> >> >> > 21:00:49.110637 IP6 (flowlabel 0xb34d5, hlim 64, next-header TCP (6) payload length: 20) fec0::5054:ff:fe12:3456.5555 > fec0::5054:ff:fe12:3456.55804: Flags [R], cksum 0x0515 (incorrect -> 0x668c), seq 1923801599, win 0, length 0
>> >> >> >> >
>> >> >> >> > The flowlabel of reset packet (0xb34d5) and flowlabel of normal packet
>> >> >> >> > (0xd827f) are different. This causes our router doesn't correctly close tcp
>> >> >> >> > connection. The patches try to fix the issue.
>> >> >> >> >
>> >> >> >> Shaohua,
>> >> >> >>
>> >> >> >> Can you give some more detail about what the router doesn't close the
>> >> >> >> TCP connection means? I'm guessing the problem is either: 1) the
>> >> >> >> router is maintaining connection state that includes the flow label in
>> >> >> >> a connection tuple. 2) some router in the path is maintaining
>> >> >> >> connection state, but when the flow label changes the flow's packet
>> >> >> >> are routed through a different router that doesn't have a state for
>> >> >> >> the flow it drops the packet. #1 should be easily fix in the router,
>> >> >> >> flow labels cannot be used as state. #2 is the known problem that
>> >> >> >> stateful firewalls have killed our ability to use multihoming.
>> >> >> >
>> >> >> > The #2 is exactly the problem we saw.
>> >> >> >
>> >> >> >> Another consideration is that sk_txhash is also used in routing
>> >> >> >> decisions by the local host (flow label is normally derived from
>> >> >> >> txhash). If you want to ensure that connections are routed
>> >> >> >> consistently for timewait state you might need sk_txhash saved also.
>> >> >> >
>> >> >> > As far as I understood, we don't use sk_txhash for routing selection. The code
>> >> >> > does routing selection with flowlabel user configured, at that time we don't
>> >> >> > derive fl6.flowlabel from skb->hash (which is from sk_txhash). The code always
>> >> >> > does routing selection first and then uses ip6_make_flowlabel to build packet
>> >> >> > data where we derive flowlabel from skb->hash.
>> >> >> >
>> >> >> That is assuming one particular use case. Generally, if you want to
>> >> >> ensure all packets for a flow take the same path you'll need tx_hash
>> >> >> and make it persistent (disable flow bender). For instance, if you
>> >> >> were doing UDP encapsulation like in VXLAN the UDP source port
>> >> >> selection is unaffected by saved flow label for the lifetime of the
>> >> >> flow. So we would still hit #2 in that case and the stateful device
>> >> >> doesn't see whole flow. It might be just as easy to move tx_hash in
>> >> >> skc_common so that it's available in TW state for this purpose. Then
>> >> >> when moving to TW state just copy the tx_hash.
>> >> >
>> >> > Hi Tom,
>> >> >
>> >> > My original implementation is to add a tx_hash in tw sock, we then copy sock's
>> >> > tx_hash to the tw tx_hash. This does makes things simplier. One concern from
>> >> > Eric is this will increase the size of tw sock. If we move tx_hash to
>> >> > skc_common, all sock size will increase, is this acceptable?
>> >>
>> >> I think that can only be measured by how critical it is to
>> >> persistently route all packets the same exact way for every
>> >> connection. Page one of the IP book clearly states that IP packets can
>> >> be dropped, duplicated, or received out of order. Received OOO implies
>> >> that packet for the same flow are allowed to take different paths. The
>> >> requirement that packets for the same flow must always take the same
>> >> path through the network was created by stateful middleboxes-- it's
>> >> not inherent in the architecture of IP networking. Unfortunately,
>> >> we're seeing this become more and more of a problem as more devices
>> >> are multi-homed (like smart phones) and these network requirement
>> >> cripple our ability to take advantage of features like that.
>> >>
>> >> Personally, I wish the middleboxes fix the problem they created, but I
>> >> suppose we need to be pragmatic at least in the short term.
>> >
>> > Hmm, I still hesitate to add a new field in skc_common. Fixing current problem
>> > looks propriate in current stage. I'd defer fixing the generic issue till it's
>> > necessary.
>> >
>> Shaohua,
>>
>> An alternative would be to not initialize sk_txhash, but instead defer
>> hash computation to use flow dissector in the TX path when the hash is
>> needed (to get flow label, src port for UDP encap, route for
>> multipath, etc.). At the first hash computation in TX path  the result
>> in sk_txhash. TW state there is no socket so flow dissector is
>> always used but that should yield the same hash. No extra fields would
>> be needed and additional cost is negligible.
>
> Hi Tom,
>
> Did you mean revert 877d1f6291f8(net: Set sk_txhash from a random number)? This
> could fix the issue. So in normal case we calculate the sk_txhash using flow
> dissector but in negative routing case we use the random hash, is this what you
> want?
>
No. What you'd want is something like a sysctl that sets an alternate
mode for sk_txhash processing. sk_txhash is derived from flow
dissector for the first TX packet and then it's never allowed to
change. Maybe this should be called persistent-hash mode.

> There seems to have other bug in this side. From my understanding, commit
> 265f94ff54d6(net: Recompute sk_txhash on negative routing advice) tries to
> select a different route. But the multipath selection code
> (rt6_multipath_select) doesn't use sk_txhash or skb->hash, it does use
> fl6.flowlabel, but that is the flowlabel user sets. So looks like the commit
> doesn't change anything.
>
The routing functions typically don't use sock of skbuff, but use flow
structs instead. It may be reasonable to add a hash to those.

> What's the 'src port for UDP encap'? I can't find the code setting skb->hash
> to sk_txhash in UDP side.
>
udp_flow_src_port is function call by UDP encaps to set source port.
This is call skb_get_hash. sk_set_txhash is function to set txhash
right now to random value. skb_set_hash_from_sk set skb->hash when
skbuff is owned by socket (skb_set_owner_w).

Thanks,
Tom


> Thanks,
> Shaohua

^ permalink raw reply

* Re: [PATCH net] ipv4: fix NULL dereference in free_fib_info_rcu()
From: David Miller @ 2017-08-16  0:14 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev
In-Reply-To: <1502799977.4936.71.camel@edumazet-glaptop3.roam.corp.google.com>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 15 Aug 2017 05:26:17 -0700

> From: Eric Dumazet <edumazet@google.com>
> 
> If fi->fib_metrics could not be allocated in fib_create_info()
> we attempt to dereference a NULL pointer in free_fib_info_rcu() :
> 
>     m = fi->fib_metrics;
>     if (m != &dst_default_metrics && atomic_dec_and_test(&m->refcnt))
>             kfree(m);
> 
> Before my recent patch, we used to call kfree(NULL) and nothing wrong
> happened.
> 
> Instead of using RCU to defer freeing while we are under memory stress,
> it seems better to take immediate action.
> 
> This was reported by syzkaller team.
> 
> Fixes: 3fb07daff8e9 ("ipv4: add reference counting to metrics")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: Dmitry Vyukov <dvyukov@google.com>

Applied and queued up for -stable.

^ permalink raw reply

* Re: [PATCH] net: Fix a typo in comment about sock flags.
From: David Miller @ 2017-08-16  0:13 UTC (permalink / raw)
  To: xiangxia.m.yue; +Cc: netdev
In-Reply-To: <1502796534-9101-1-git-send-email-xiangxia.m.yue@gmail.com>

From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
Date: Tue, 15 Aug 2017 04:28:54 -0700

> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>

Applied.

^ permalink raw reply

* Re: [PATCH net] ipv6: fix NULL dereference in ip6_route_dev_notify()
From: David Miller @ 2017-08-16  0:06 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev
In-Reply-To: <1502795391.4936.60.camel@edumazet-glaptop3.roam.corp.google.com>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 15 Aug 2017 04:09:51 -0700

> From: Eric Dumazet <edumazet@google.com>
> 
> Based on a syzkaller report [1], I found that a per cpu allocation
> failure in snmp6_alloc_dev() would then lead to NULL dereference in
> ip6_route_dev_notify().
> 
> It seems this is a very old bug, thus no Fixes tag in this submission.
> 
> Let's add in6_dev_put_clear() helper, as we will probably use
> it elsewhere (once available/present in net-next)
 ...
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: Dmitry Vyukov <dvyukov@google.com>

Applied and queued up for -stable, thanks Eric.

^ permalink raw reply

* Re: [patch net-next] mlxsw: spectrum_router: Use correct config option
From: David Miller @ 2017-08-16  0:05 UTC (permalink / raw)
  To: jiri; +Cc: netdev, idosch, mlxsw
In-Reply-To: <20170815071033.1420-1-jiri@resnulli.us>

From: Jiri Pirko <jiri@resnulli.us>
Date: Tue, 15 Aug 2017 09:10:33 +0200

> From: Ido Schimmel <idosch@mellanox.com>
> 
> I made an embarrassing mistake and used 'IPV6' instead of 'CONFIG_IPV6'
> around the function that updates the kernel about IPv6 neighbours
> activity. This can be a problem if the kernel has more neighbours than a
> certain threshold and it starts deleting those that are supposedly
> inactive.
> 
> Fixes: b5f3e0d43012 ("mlxsw: spectrum_router: Fix build when IPv6 isn't enabled")
> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>

Also applied, thanks.

^ permalink raw reply

* Re: [patch net-next] ipv6: fib: Provide offload indication using nexthop flags
From: David Miller @ 2017-08-16  0:05 UTC (permalink / raw)
  To: jiri
  Cc: netdev, idosch, mlxsw, dsahern, roopa, nikolay, hannes, yoshfuji,
	kuznet
In-Reply-To: <20170815070949.1363-1-jiri@resnulli.us>

From: Jiri Pirko <jiri@resnulli.us>
Date: Tue, 15 Aug 2017 09:09:49 +0200

> From: Ido Schimmel <idosch@mellanox.com>
> 
> IPv6 routes currently lack nexthop flags as in IPv4. This has several
> implications.
> 
> In the forwarding path, it requires us to check the carrier state of the
> nexthop device and potentially ignore a linkdown route, instead of
> checking for RTNH_F_LINKDOWN.
> 
> It also requires capable drivers to use the user facing IPv6-specific
> route flags to provide offload indication, instead of using the nexthop
> flags as in IPv4.
> 
> Add nexthop flags to IPv6 routes in the 40 bytes hole and use it to
> provide offload indication instead of the RTF_OFFLOAD flag, which is
> removed while it's still not part of any official kernel release.
> 
> In the near future we would like to use the field for the
> RTNH_F_{LINKDOWN,DEAD} flags, but this change is more involved and might
> not be ready in time for the current cycle.
> 
> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>

Applied.

^ permalink raw reply

* Re: [PATCH v4 iproute2 1/7] rdma: Add basic infrastructure for RDMA tool
From: Stephen Hemminger @ 2017-08-15 23:54 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: Doug Ledford, linux-rdma, Dennis Dalessandro, Jason Gunthorpe,
	Jiri Pirko, Ariel Almog, Linux Netdev
In-Reply-To: <20170815130020.29509-2-leonro@mellanox.com>

On Tue, 15 Aug 2017 16:00:14 +0300
Leon Romanovsky <leonro@mellanox.com> wrote:

> RDMA devices are cross-functional devices from one side,
> but very tailored for the specific markets from another.
> 
> Such diversity caused to spread of RDMA related configuration
> across various tools, e.g. devlink, ip, ethtool, ib specific and
> vendor specific solutions.
> 
> This patch adds ability to fill device and port information
> by reading RDMA netlink.
> 
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>

Won't build for me.

$ make distclean

$ make

In file included from rdma.c:12:0:
rdma.h:23:23: fatal error: rdma/rdma.h: No such file or directory
 #include <rdma/rdma.h>
                       ^

I think your Makefile expects something different.
Or more Config fallout

^ permalink raw reply

* linux-next: manual merge of the pci tree with the net tree
From: Stephen Rothwell @ 2017-08-15 23:51 UTC (permalink / raw)
  To: Bjorn Helgaas, David Miller, Networking
  Cc: Linux-Next Mailing List, Linux Kernel Mailing List, Ding Tianhong,
	Casey Leedom, Sinan Kaya

Hi Bjorn,

Today's linux-next merge of the pci tree got a conflict in:

  drivers/pci/probe.c

between commit:

  a99b646afa8a ("PCI: Disable PCIe Relaxed Ordering if unsupported")

from the net tree and commit:

  62ce94a7a5a5 ("PCI: Mark Broadcom HT2100 Root Port Extended Tags as broken")

from the pci tree.

I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

-- 
Cheers,
Stephen Rothwell

diff --cc drivers/pci/probe.c
index e6a917b4acd3,d11fede6bd53..000000000000
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@@ -1751,67 -1753,51 +1753,94 @@@ int pci_configure_extended_tags(struct 
  	int ret;
  
  	if (!pci_is_pcie(dev))
- 		return;
+ 		return 0;
  
- 	ret = pcie_capability_read_dword(dev, PCI_EXP_DEVCAP, &dev_cap);
+ 	ret = pcie_capability_read_dword(dev, PCI_EXP_DEVCAP, &cap);
  	if (ret)
- 		return;
+ 		return 0;
+ 
+ 	if (!(cap & PCI_EXP_DEVCAP_EXT_TAG))
+ 		return 0;
+ 
+ 	ret = pcie_capability_read_word(dev, PCI_EXP_DEVCTL, &ctl);
+ 	if (ret)
+ 		return 0;
+ 
+ 	host = pci_find_host_bridge(dev->bus);
+ 	if (!host)
+ 		return 0;
+ 
+ 	/*
+ 	 * If some device in the hierarchy doesn't handle Extended Tags
+ 	 * correctly, make sure they're disabled.
+ 	 */
+ 	if (host->no_ext_tags) {
+ 		if (ctl & PCI_EXP_DEVCTL_EXT_TAG) {
+ 			dev_info(&dev->dev, "disabling Extended Tags\n");
+ 			pcie_capability_clear_word(dev, PCI_EXP_DEVCTL,
+ 						   PCI_EXP_DEVCTL_EXT_TAG);
+ 		}
+ 		return 0;
+ 	}
  
- 	if (dev_cap & PCI_EXP_DEVCAP_EXT_TAG)
+ 	if (!(ctl & PCI_EXP_DEVCTL_EXT_TAG)) {
+ 		dev_info(&dev->dev, "enabling Extended Tags\n");
  		pcie_capability_set_word(dev, PCI_EXP_DEVCTL,
  					 PCI_EXP_DEVCTL_EXT_TAG);
+ 	}
+ 	return 0;
  }
  
 +/**
 + * pcie_relaxed_ordering_enabled - Probe for PCIe relaxed ordering enable
 + * @dev: PCI device to query
 + *
 + * Returns true if the device has enabled relaxed ordering attribute.
 + */
 +bool pcie_relaxed_ordering_enabled(struct pci_dev *dev)
 +{
 +	u16 v;
 +
 +	pcie_capability_read_word(dev, PCI_EXP_DEVCTL, &v);
 +
 +	return !!(v & PCI_EXP_DEVCTL_RELAX_EN);
 +}
 +EXPORT_SYMBOL(pcie_relaxed_ordering_enabled);
 +
 +static void pci_configure_relaxed_ordering(struct pci_dev *dev)
 +{
 +	struct pci_dev *root;
 +
 +	/* PCI_EXP_DEVICE_RELAX_EN is RsvdP in VFs */
 +	if (dev->is_virtfn)
 +		return;
 +
 +	if (!pcie_relaxed_ordering_enabled(dev))
 +		return;
 +
 +	/*
 +	 * For now, we only deal with Relaxed Ordering issues with Root
 +	 * Ports. Peer-to-Peer DMA is another can of worms.
 +	 */
 +	root = pci_find_pcie_root_port(dev);
 +	if (!root)
 +		return;
 +
 +	if (root->dev_flags & PCI_DEV_FLAGS_NO_RELAXED_ORDERING) {
 +		pcie_capability_clear_word(dev, PCI_EXP_DEVCTL,
 +					   PCI_EXP_DEVCTL_RELAX_EN);
 +		dev_info(&dev->dev, "Disable Relaxed Ordering because the Root Port didn't support it\n");
 +	}
 +}
 +
  static void pci_configure_device(struct pci_dev *dev)
  {
  	struct hotplug_params hpp;
  	int ret;
  
  	pci_configure_mps(dev);
- 	pci_configure_extended_tags(dev);
+ 	pci_configure_extended_tags(dev, NULL);
 +	pci_configure_relaxed_ordering(dev);
  
  	memset(&hpp, 0, sizeof(hpp));
  	ret = pci_get_hp_params(dev, &hpp);

^ permalink raw reply

* Re: [PATCH 2/2] mlx5: remove unnecessary pci_set_drvdata()
From: David Miller @ 2017-08-15 23:47 UTC (permalink / raw)
  To: yanjun.zhu-QHcLZuEGTsvQT0dZR+AlfA
  Cc: saeedm-VPRAkNaXOzVWk0Htik3J/w, matanb-VPRAkNaXOzVWk0Htik3J/w,
	leonro-VPRAkNaXOzVWk0Htik3J/w, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	yuval.shaia-QHcLZuEGTsvQT0dZR+AlfA
In-Reply-To: <1502778786-14738-2-git-send-email-yanjun.zhu-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>

From: Zhu Yanjun <yanjun.zhu-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
Date: Tue, 15 Aug 2017 02:33:06 -0400

> The driver core clears the driver data to NULL after device_release
> or on probe failure. Thus, it is not necessary to manually clear the
> device driver data to NULL.
> 
> Cc: Joe Jin <joe.jin-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> Cc: Junxiao Bi <junxiao.bi-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Zhu Yanjun <yanjun.zhu-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>

Applied.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 1/2] mlx4: remove unnecessary pci_set_drvdata()
From: David Miller @ 2017-08-15 23:46 UTC (permalink / raw)
  To: yanjun.zhu-QHcLZuEGTsvQT0dZR+AlfA
  Cc: saeedm-VPRAkNaXOzVWk0Htik3J/w, matanb-VPRAkNaXOzVWk0Htik3J/w,
	leonro-VPRAkNaXOzVWk0Htik3J/w, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	yuval.shaia-QHcLZuEGTsvQT0dZR+AlfA
In-Reply-To: <1502778786-14738-1-git-send-email-yanjun.zhu-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>

From: Zhu Yanjun <yanjun.zhu-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
Date: Tue, 15 Aug 2017 02:33:05 -0400

> The driver core clears the driver data to NULL after device_release
> or on probe failure. Thus, it is not necessary to manually clear the
> device driver data to NULL.
> 
> Cc: Joe Jin <joe.jin-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> Cc: Junxiao Bi <junxiao.bi-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Zhu Yanjun <yanjun.zhu-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>

Applied.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH net] bpf: fix bpf_trace_printk on 32 bit archs
From: Daniel Borkmann @ 2017-08-15 23:45 UTC (permalink / raw)
  To: davem; +Cc: james.hogan, ast, netdev, Daniel Borkmann

James reported that on MIPS32 bpf_trace_printk() is currently
broken while MIPS64 works fine:

  bpf_trace_printk() uses conditional operators to attempt to
  pass different types to __trace_printk() depending on the
  format operators. This doesn't work as intended on 32-bit
  architectures where u32 and long are passed differently to
  u64, since the result of C conditional operators follows the
  "usual arithmetic conversions" rules, such that the values
  passed to __trace_printk() will always be u64 [causing issues
  later in the va_list handling for vscnprintf()].

  For example the samples/bpf/tracex5 test printed lines like
  below on MIPS32, where the fd and buf have come from the u64
  fd argument, and the size from the buf argument:

    [...] 1180.941542: 0x00000001: write(fd=1, buf=  (null), size=6258688)

  Instead of this:

    [...] 1625.616026: 0x00000001: write(fd=1, buf=009e4000, size=512)

One way to get it working is to expand various combinations
of argument types into 8 different combinations for 32 bit
and 64 bit kernels. Fix tested by James on MIPS32 and MIPS64
as well that it resolves the issue.

Fixes: 9c959c863f82 ("tracing: Allow BPF programs to call bpf_trace_printk()")
Reported-by: James Hogan <james.hogan@imgtec.com>
Tested-by: James Hogan <james.hogan@imgtec.com>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
 kernel/trace/bpf_trace.c | 34 ++++++++++++++++++++++++++++++----
 1 file changed, 30 insertions(+), 4 deletions(-)

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 3738519..dc498b6 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -204,10 +204,36 @@ static const struct bpf_func_proto *bpf_get_probe_write_proto(void)
 		fmt_cnt++;
 	}
 
-	return __trace_printk(1/* fake ip will not be printed */, fmt,
-			      mod[0] == 2 ? arg1 : mod[0] == 1 ? (long) arg1 : (u32) arg1,
-			      mod[1] == 2 ? arg2 : mod[1] == 1 ? (long) arg2 : (u32) arg2,
-			      mod[2] == 2 ? arg3 : mod[2] == 1 ? (long) arg3 : (u32) arg3);
+/* Horrid workaround for getting va_list handling working with different
+ * argument type combinations generically for 32 and 64 bit archs.
+ */
+#define __BPF_TP_EMIT()	__BPF_ARG3_TP()
+#define __BPF_TP(...)							\
+	__trace_printk(1 /* Fake ip will not be printed. */,		\
+		       fmt, ##__VA_ARGS__)
+
+#define __BPF_ARG1_TP(...)						\
+	((mod[0] == 2 || (mod[0] == 1 && __BITS_PER_LONG == 64))	\
+	  ? __BPF_TP(arg1, ##__VA_ARGS__)				\
+	  : ((mod[0] == 1 || (mod[0] == 0 && __BITS_PER_LONG == 32))	\
+	      ? __BPF_TP((long)arg1, ##__VA_ARGS__)			\
+	      : __BPF_TP((u32)arg1, ##__VA_ARGS__)))
+
+#define __BPF_ARG2_TP(...)						\
+	((mod[1] == 2 || (mod[1] == 1 && __BITS_PER_LONG == 64))	\
+	  ? __BPF_ARG1_TP(arg2, ##__VA_ARGS__)				\
+	  : ((mod[1] == 1 || (mod[1] == 0 && __BITS_PER_LONG == 32))	\
+	      ? __BPF_ARG1_TP((long)arg2, ##__VA_ARGS__)		\
+	      : __BPF_ARG1_TP((u32)arg2, ##__VA_ARGS__)))
+
+#define __BPF_ARG3_TP(...)						\
+	((mod[2] == 2 || (mod[2] == 1 && __BITS_PER_LONG == 64))	\
+	  ? __BPF_ARG2_TP(arg3, ##__VA_ARGS__)				\
+	  : ((mod[2] == 1 || (mod[2] == 0 && __BITS_PER_LONG == 32))	\
+	      ? __BPF_ARG2_TP((long)arg3, ##__VA_ARGS__)		\
+	      : __BPF_ARG2_TP((u32)arg3, ##__VA_ARGS__)))
+
+	return __BPF_TP_EMIT();
 }
 
 static const struct bpf_func_proto bpf_trace_printk_proto = {
-- 
1.9.3

^ permalink raw reply related

* Re: [PATCH iproute2 net-next v2 0/3] Add support for SRv6 local segment processing
From: Stephen Hemminger @ 2017-08-15 23:45 UTC (permalink / raw)
  To: David Lebrun; +Cc: netdev
In-Reply-To: <20170809153326.29599-1-david.lebrun@uclouvain.be>

On Wed, 9 Aug 2017 17:33:23 +0200
David Lebrun <david.lebrun@uclouvain.be> wrote:

> This patch series adds support and documentation for the seg6local
> lightweight tunnel, enabling to perform operations to SR-enabled packets
> based on their active segment.
> 
> v2: use a table for action names
> 
> Signed-off-by: David Lebrun <david.lebrun@uclouvain.be>
> 
> David Lebrun (3):
>   iproute: add helper functions for SRH processing
>   iproute: add support for SRv6 local segment processing
>   man: add documentation for seg6local lwt
> 
>  ip/iproute.c           |   2 +-
>  ip/iproute_lwtunnel.c  | 324 +++++++++++++++++++++++++++++++++++++++++--------
>  man/man8/ip-route.8.in |  62 +++++++++-
>  3 files changed, 338 insertions(+), 50 deletions(-)
> 

Applied to net-next branch.
Thanks David.

^ permalink raw reply

* Re: [PATCH net V2] openvswitch: fix skb_panic due to the incorrect actions attrlen
From: Pravin Shelar @ 2017-08-15 23:35 UTC (permalink / raw)
  To: Liping Zhang
  Cc: Pravin Shelar, David S. Miller, Linux Kernel Network Developers,
	Liping Zhang, Neil McKee
In-Reply-To: <20170815112913.14678-1-zlpnobody@163.com>

On Tue, Aug 15, 2017 at 4:29 AM, Liping Zhang <zlpnobody@163.com> wrote:
> From: Liping Zhang <zlpnobody@gmail.com>
>
> For sw_flow_actions, the actions_len only represents the kernel part's
> size, and when we dump the actions to the userspace, we will do the
> convertions, so it's true size may become bigger than the actions_len.
>
> But unfortunately, for OVS_PACKET_ATTR_ACTIONS, we use the actions_len
> to alloc the skbuff, so the user_skb's size may become insufficient and
> oops will happen like this:
>   skbuff: skb_over_panic: text:ffffffff8148fabf len:1749 put:157 head:
>   ffff881300f39000 data:ffff881300f39000 tail:0x6d5 end:0x6c0 dev:<NULL>
>   ------------[ cut here ]------------
>   kernel BUG at net/core/skbuff.c:129!
>   [...]
>   Call Trace:
>    <IRQ>
>    [<ffffffff8148be82>] skb_put+0x43/0x44
>    [<ffffffff8148fabf>] skb_zerocopy+0x6c/0x1f4
>    [<ffffffffa0290d36>] queue_userspace_packet+0x3a3/0x448 [openvswitch]
>    [<ffffffffa0292023>] ovs_dp_upcall+0x30/0x5c [openvswitch]
>    [<ffffffffa028d435>] output_userspace+0x132/0x158 [openvswitch]
>    [<ffffffffa01e6890>] ? ip6_rcv_finish+0x74/0x77 [ipv6]
>    [<ffffffffa028e277>] do_execute_actions+0xcc1/0xdc8 [openvswitch]
>    [<ffffffffa028e3f2>] ovs_execute_actions+0x74/0x106 [openvswitch]
>    [<ffffffffa0292130>] ovs_dp_process_packet+0xe1/0xfd [openvswitch]
>    [<ffffffffa0292b77>] ? key_extract+0x63c/0x8d5 [openvswitch]
>    [<ffffffffa029848b>] ovs_vport_receive+0xa1/0xc3 [openvswitch]
>   [...]
>
> Also we can find that the actions_len is much little than the orig_len:
>   crash> struct sw_flow_actions 0xffff8812f539d000
>   struct sw_flow_actions {
>     rcu = {
>       next = 0xffff8812f5398800,
>       func = 0xffffe3b00035db32
>     },
>     orig_len = 1384,
>     actions_len = 592,
>     actions = 0xffff8812f539d01c
>   }
>
> So as a quick fix, use the orig_len instead of the actions_len to alloc
> the user_skb.
>
> Last, this oops happened on our system running a relative old kernel, but
> the same risk still exists on the mainline, since we use the wrong
> actions_len from the beginning.
>
> Fixes: ccea74457bbd ("openvswitch: include datapath actions with sampled-packet upcall to userspace")
> Cc: Neil McKee <neil.mckee@inmon.com>
> Signed-off-by: Liping Zhang <zlpnobody@gmail.com>
> ---
>  V2: move actions_attrlen into ovs_skb_cb, which will make codes more
>      clean, suggested by Pravin Shelar.
>
>  net/openvswitch/actions.c  | 2 ++
>  net/openvswitch/datapath.c | 2 +-
>  net/openvswitch/datapath.h | 3 +++
>  3 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
> index e4610676299b..f849ef52853f 100644
> --- a/net/openvswitch/actions.c
> +++ b/net/openvswitch/actions.c
> @@ -921,6 +921,7 @@ static int output_userspace(struct datapath *dp, struct sk_buff *skb,
>                         /* Include actions. */
>                         upcall.actions = actions;
>                         upcall.actions_len = actions_len;
> +                       upcall.actions_attrlen = OVS_CB(skb)->acts_origlen;
OVS_CB acts_origlen should be accessible in upcall_msg_size (), is
there reason to add this member to struct dp_upcall_info?

^ permalink raw reply

* Re: [PATCH v3 net-next] bpf/verifier: track liveness for pruning
From: David Miller @ 2017-08-15 23:32 UTC (permalink / raw)
  To: daniel; +Cc: ecree, alexei.starovoitov, ast, netdev, linux-kernel, iovisor-dev
In-Reply-To: <599371EA.20605@iogearbox.net>

From: Daniel Borkmann <daniel@iogearbox.net>
Date: Wed, 16 Aug 2017 00:12:58 +0200

> On 08/15/2017 09:34 PM, Edward Cree wrote:
>> State of a register doesn't matter if it wasn't read in reaching an
>> exit;
>>   a write screens off all reads downstream of it from all
>>   explored_states
>>   upstream of it.
>> This allows us to prune many more branches; here are some processed
>> insn
>>   counts for some Cilium programs:
>> Program                  before  after
>> bpf_lb_opt_-DLB_L3.o       6515   3361
>> bpf_lb_opt_-DLB_L4.o       8976   5176
>> bpf_lb_opt_-DUNKNOWN.o     2960   1137
>> bpf_lxc_opt_-DDROP_ALL.o  95412  48537
>> bpf_lxc_opt_-DUNKNOWN.o  141706  78718
>> bpf_netdev.o              24251  17995
>> bpf_overlay.o             10999   9385
>>
>> The runtime is also improved; here are 'time' results in ms:
>> Program                  before  after
>> bpf_lb_opt_-DLB_L3.o         24      6
>> bpf_lb_opt_-DLB_L4.o         26     11
>> bpf_lb_opt_-DUNKNOWN.o       11      2
>> bpf_lxc_opt_-DDROP_ALL.o   1288    139
>> bpf_lxc_opt_-DUNKNOWN.o    1768    234
>> bpf_netdev.o                 62     31
>> bpf_overlay.o                15     13
>>
>> Signed-off-by: Edward Cree <ecree@solarflare.com>
> 
> Acked-by: Daniel Borkmann <daniel@iogearbox.net>

Applied, nice work Edward.

^ permalink raw reply


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