* Re: [RFC PATCH 00/24] Introducing AF_XDP support
From: Tushar Dave @ 2018-03-26 22:54 UTC (permalink / raw)
To: Jesper Dangaard Brouer, William Tu
Cc: Björn Töpel, magnus.karlsson, Alexander Duyck,
Alexander Duyck, John Fastabend, Alexei Starovoitov,
willemdebruijn.kernel, Daniel Borkmann,
Linux Kernel Network Developers, Björn Töpel,
michael.lundkvist, jesse.brandeburg, anjali.singhai,
jeffrey.b.shaw, ferruh.yigit, qi.z.zhang
In-Reply-To: <20180326183810.2ef4e29f@redhat.com>
On 03/26/2018 09:38 AM, Jesper Dangaard Brouer wrote:
>
> On Mon, 26 Mar 2018 09:06:54 -0700 William Tu <u9012063@gmail.com> wrote:
>
>> On Wed, Jan 31, 2018 at 5:53 AM, Björn Töpel <bjorn.topel@gmail.com> wrote:
>>> From: Björn Töpel <bjorn.topel@intel.com>
>>>
>>> This RFC introduces a new address family called AF_XDP that is
>>> optimized for high performance packet processing and zero-copy
>>> semantics. Throughput improvements can be up to 20x compared to V2 and
>>> V3 for the micro benchmarks included. Would be great to get your
>>> feedback on it. Note that this is the follow up RFC to AF_PACKET V4
>>> from November last year. The feedback from that RFC submission and the
>>> presentation at NetdevConf in Seoul was to create a new address family
>>> instead of building on top of AF_PACKET. AF_XDP is this new address
>>> family.
>>>
>>> The main difference between AF_XDP and AF_PACKET V2/V3 on a descriptor
>>> level is that TX and RX descriptors are separated from packet
>>> buffers. An RX or TX descriptor points to a data buffer in a packet
>>> buffer area. RX and TX can share the same packet buffer so that a
>>> packet does not have to be copied between RX and TX. Moreover, if a
>>> packet needs to be kept for a while due to a possible retransmit, then
>>> the descriptor that points to that packet buffer can be changed to
>>> point to another buffer and reused right away. This again avoids
>>> copying data.
>>>
>>> The RX and TX descriptor rings are registered with the setsockopts
>>> XDP_RX_RING and XDP_TX_RING, similar to AF_PACKET. The packet buffer
>>> area is allocated by user space and registered with the kernel using
>>> the new XDP_MEM_REG setsockopt. All these three areas are shared
>>> between user space and kernel space. The socket is then bound with a
>>> bind() call to a device and a specific queue id on that device, and it
>>> is not until bind is completed that traffic starts to flow.
>>>
>>> An XDP program can be loaded to direct part of the traffic on that
>>> device and queue id to user space through a new redirect action in an
>>> XDP program called bpf_xdpsk_redirect that redirects a packet up to
>>> the socket in user space. All the other XDP actions work just as
>>> before. Note that the current RFC requires the user to load an XDP
>>> program to get any traffic to user space (for example all traffic to
>>> user space with the one-liner program "return
>>> bpf_xdpsk_redirect();"). We plan on introducing a patch that removes
>>> this requirement and sends all traffic from a queue to user space if
>>> an AF_XDP socket is bound to it.
>>>
>>> AF_XDP can operate in three different modes: XDP_SKB, XDP_DRV, and
>>> XDP_DRV_ZC (shorthand for XDP_DRV with a zero-copy allocator as there
>>> is no specific mode called XDP_DRV_ZC). If the driver does not have
>>> support for XDP, or XDP_SKB is explicitly chosen when loading the XDP
>>> program, XDP_SKB mode is employed that uses SKBs together with the
>>> generic XDP support and copies out the data to user space. A fallback
>>> mode that works for any network device. On the other hand, if the
>>> driver has support for XDP (all three NDOs: ndo_bpf, ndo_xdp_xmit and
>>> ndo_xdp_flush), these NDOs, without any modifications, will be used by
>>> the AF_XDP code to provide better performance, but there is still a
>>> copy of the data into user space. The last mode, XDP_DRV_ZC, is XDP
>>> driver support with the zero-copy user space allocator that provides
>>> even better performance. In this mode, the networking HW (or SW driver
>>> if it is a virtual driver like veth) DMAs/puts packets straight into
>>> the packet buffer that is shared between user space and kernel
>>> space. The RX and TX descriptor queues of the networking HW are NOT
>>> shared to user space. Only the kernel can read and write these and it
>>> is the kernel driver's responsibility to translate these HW specific
>>> descriptors to the HW agnostic ones in the virtual descriptor rings
>>> that user space sees. This way, a malicious user space program cannot
>>> mess with the networking HW. This mode though requires some extensions
>>> to XDP.
>>>
>>> To get the XDP_DRV_ZC mode to work for RX, we chose to introduce a
>>> buffer pool concept so that the same XDP driver code can be used for
>>> buffers allocated using the page allocator (XDP_DRV), the user-space
>>> zero-copy allocator (XDP_DRV_ZC), or some internal driver specific
>>> allocator/cache/recycling mechanism. The ndo_bpf call has also been
>>> extended with two commands for registering and unregistering an XSK
>>> socket and is in the RX case mainly used to communicate some
>>> information about the user-space buffer pool to the driver.
>>>
>>> For the TX path, our plan was to use ndo_xdp_xmit and ndo_xdp_flush,
>>> but we run into problems with this (further discussion in the
>>> challenges section) and had to introduce a new NDO called
>>> ndo_xdp_xmit_xsk (xsk = XDP socket). It takes a pointer to a netdevice
>>> and an explicit queue id that packets should be sent out on. In
>>> contrast to ndo_xdp_xmit, it is asynchronous and pulls packets to be
>>> sent from the xdp socket (associated with the dev and queue
>>> combination that was provided with the NDO call) using a callback
>>> (get_tx_packet), and when they have been transmitted it uses another
>>> callback (tx_completion) to signal completion of packets. These
>>> callbacks are set via ndo_bpf in the new XDP_REGISTER_XSK
>>> command. ndo_xdp_xmit_xsk is exclusively used by the XDP socket code
>>> and thus does not clash with the XDP_REDIRECT use of
>>> ndo_xdp_xmit. This is one of the reasons that the XDP_DRV mode
>>> (without ZC) is currently not supported by TX. Please have a look at
>>> the challenges section for further discussions.
>>>
>>> The AF_XDP bind call acts on a queue pair (channel in ethtool speak),
>>> so the user needs to steer the traffic to the zero-copy enabled queue
>>> pair. Which queue to use, is up to the user.
>>>
>>> For an untrusted application, HW packet steering to a specific queue
>>> pair (the one associated with the application) is a requirement, as
>>> the application would otherwise be able to see other user space
>>> processes' packets. If the HW cannot support the required packet
>>> steering, XDP_DRV or XDP_SKB mode have to be used as they do not
>>> expose the NIC's packet buffer into user space as the packets are
>>> copied into user space from the NIC's packet buffer in the kernel.
>>>
>>> There is a xdpsock benchmarking/test application included. Say that
>>> you would like your UDP traffic from port 4242 to end up in queue 16,
>>> that we will enable AF_XDP on. Here, we use ethtool for this:
>>>
>>> ethtool -N p3p2 rx-flow-hash udp4 fn
>>> ethtool -N p3p2 flow-type udp4 src-port 4242 dst-port 4242 \
>>> action 16
>>>
>>> Running the l2fwd benchmark in XDP_DRV_ZC mode can then be done using:
>>>
>>> samples/bpf/xdpsock -i p3p2 -q 16 -l -N
>>>
>>> For XDP_SKB mode, use the switch "-S" instead of "-N" and all options
>>> can be displayed with "-h", as usual.
>>>
>>> We have run some benchmarks on a dual socket system with two Broadwell
>>> E5 2660 @ 2.0 GHz with hyperthreading turned off. Each socket has 14
>>> cores which gives a total of 28, but only two cores are used in these
>>> experiments. One for TR/RX and one for the user space application. The
>>> memory is DDR4 @ 2133 MT/s (1067 MHz) and the size of each DIMM is
>>> 8192MB and with 8 of those DIMMs in the system we have 64 GB of total
>>> memory. The compiler used is gcc version 5.4.0 20160609. The NIC is an
>>> Intel I40E 40Gbit/s using the i40e driver.
>>>
>>> Below are the results in Mpps of the I40E NIC benchmark runs for 64
>>> byte packets, generated by commercial packet generator HW that is
>>> generating packets at full 40 Gbit/s line rate.
>>>
>>> XDP baseline numbers without this RFC:
>>> xdp_rxq_info --action XDP_DROP 31.3 Mpps
>>> xdp_rxq_info --action XDP_TX 16.7 Mpps
>>>
>>> XDP performance with this RFC i.e. with the buffer allocator:
>>> XDP_DROP 21.0 Mpps
>>> XDP_TX 11.9 Mpps
>>>
>>> AF_PACKET V4 performance from previous RFC on 4.14-rc7:
>>> Benchmark V2 V3 V4 V4+ZC
>>> rxdrop 0.67 0.73 0.74 33.7
>>> txpush 0.98 0.98 0.91 19.6
>>> l2fwd 0.66 0.71 0.67 15.5
>>>
>>> AF_XDP performance:
>>> Benchmark XDP_SKB XDP_DRV XDP_DRV_ZC (all in Mpps)
>>> rxdrop 3.3 11.6 16.9
>>> txpush 2.2 NA* 21.8
>>> l2fwd 1.7 NA* 10.4
>>>
>>
>> Hi,
>> I also did an evaluation of AF_XDP, however the performance isn't as
>> good as above.
>> I'd like to share the result and see if there are some tuning suggestions.
>>
>> System:
>> 16 core, Intel(R) Xeon(R) CPU E5-2440 v2 @ 1.90GHz
>> Intel 10G X540-AT2 ---> so I can only run XDP_SKB mode
>
> Hmmm, why is X540-AT2 not able to use XDP natively?
>
>> AF_XDP performance:
>> Benchmark XDP_SKB
>> rxdrop 1.27 Mpps
>> txpush 0.99 Mpps
>> l2fwd 0.85 Mpps
>
> Definitely too low...
>
> What is the performance if you drop packets via iptables?
>
> Command:
> $ iptables -t raw -I PREROUTING -p udp --dport 9 --j DROP
>
>> NIC configuration:
>> the command
>> "ethtool -N p3p2 flow-type udp4 src-port 4242 dst-port 4242 action 16"
>> doesn't work on my ixgbe driver, so I use ntuple:
>>
>> ethtool -K enp10s0f0 ntuple on
>> ethtool -U enp10s0f0 flow-type udp4 src-ip 10.1.1.100 action 1
>> then
>> echo 1 > /proc/sys/net/core/bpf_jit_enable
>> ./xdpsock -i enp10s0f0 -r -S --queue=1
>>
>> I also take a look at perf result:
>> For rxdrop:
>> 86.56% xdpsock xdpsock [.] main
>> 9.22% xdpsock [kernel.vmlinux] [k] nmi
>> 4.23% xdpsock xdpsock [.] xq_enq
>
> It looks very strange that you see non-maskable interrupt's (NMI) being
> this high...
>
>
>> For l2fwd:
>> 20.81% xdpsock xdpsock [.] main
>> 10.64% xdpsock [kernel.vmlinux] [k] clflush_cache_range
>
> Oh, clflush_cache_range is being called!
> Do your system use an IOMMU ?
Whats the implication here. Should IOMMU be disabled?
I'm asking because I do see a huge difference while running pktgen test
for my performance benchmarks, with and without intel_iommu.
-Tushar
>
>> 8.46% xdpsock [kernel.vmlinux] [k] xsk_sendmsg
>> 6.72% xdpsock [kernel.vmlinux] [k] skb_set_owner_w
>> 5.89% xdpsock [kernel.vmlinux] [k] __domain_mapping
>> 5.74% xdpsock [kernel.vmlinux] [k] alloc_skb_with_frags
>> 4.62% xdpsock [kernel.vmlinux] [k] netif_skb_features
>> 3.96% xdpsock [kernel.vmlinux] [k] ___slab_alloc
>> 3.18% xdpsock [kernel.vmlinux] [k] nmi
>
> Again high count for NMI ?!?
>
> Maybe you just forgot to tell perf that you want it to decode the
> bpf_prog correctly?
>
> https://prototype-kernel.readthedocs.io/en/latest/bpf/troubleshooting.html#perf-tool-symbols
>
> Enable via:
> $ sysctl net/core/bpf_jit_kallsyms=1
>
> And use perf report (while BPF is STILL LOADED):
>
> $ perf report --kallsyms=/proc/kallsyms
>
> E.g. for emailing this you can use this command:
>
> $ perf report --sort cpu,comm,dso,symbol --kallsyms=/proc/kallsyms --no-children --stdio -g none | head -n 40
>
>
>> I observed that the i40e's XDP_SKB result is much better than my ixgbe's result.
>> I wonder in XDP_SKB mode, does the driver make performance difference?
>> Or my cpu (E5-2440 v2 @ 1.90GHz) is too old?
>
> I suspect some setup issue on your system.
>
^ permalink raw reply
* Re: [net-next 00/15][pull request] 100GbE Intel Wired LAN Driver Updates 2018-03-26
From: David Miller @ 2018-03-26 22:54 UTC (permalink / raw)
To: jeffrey.t.kirsher
Cc: netdev, nhorman, sassmann, jogreene, tbogendoerfer, bpoirier
In-Reply-To: <20180326194619.1202-1-jeffrey.t.kirsher@intel.com>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Mon, 26 Mar 2018 12:46:04 -0700
> This patch series adds the ice driver, which will support the Intel(R)
> E800 Series of network devices.
Pulled, thanks Jeff.
^ permalink raw reply
* Re: [PATCH net-next 4/8] dt-bindings: net: add DT bindings for Microsemi Ocelot Switch
From: Andrew Lunn @ 2018-03-26 22:50 UTC (permalink / raw)
To: Rob Herring
Cc: Florian Fainelli, Alexandre Belloni, David S . Miller,
Allan Nielsen, razvan.stefanescu, po.liu, Thomas Petazzoni,
netdev, devicetree, linux-kernel, linux-mips
In-Reply-To: <20180326222514.4eciw66aihhcjgtw@rob-hp-laptop>
> ports and port collide with the OF graph binding. It would be good if
> this moved to ethernet-port(s) or similar.
Hi Rob
Well, we have been using port in DSA since March 2013. ports is a bit
newer, June 2016.
Changing DSA is not going to happen. But new switch bindings could use
ethernet-port(s). It just makes them inconsistent with existing switch
drivers.
Andrew
^ permalink raw reply
* Re: [RFC PATCH v2] net: phy: Added device tree binding for dev-addr and dev-addr code check-up
From: Florian Fainelli @ 2018-03-26 22:44 UTC (permalink / raw)
To: vicentiu.galanopulo, netdev, linux-kernel, robh+dt, mark.rutland,
davem, marcel, devicetree
Cc: madalin.bucur, alexandru.marginean
In-Reply-To: <20180323150522.9603-1-vicentiu.galanopulo@nxp.com>
On 03/23/2018 08:05 AM, Vicentiu Galanopulo wrote:
> Reason for this patch is that the Inphi PHY has a
> vendor specific address space for accessing the
> C45 MDIO registers - starting from 0x1e.
>
> A search of the dev-addr property is done in of_mdiobus_register.
> If the property is found in the PHY node,
> of_mdiobus_register_static_phy is called. This is a
> wrapper function for of_mdiobus_register_phy which finds the
> device in package based on dev-addr and fills devices_addrs:
> devices_addrs is a new field added to phy_c45_device_ids.
> This new field will store the dev-addr property on the same
> index where the device in package has been found.
> In order to have dev-addr in get_phy_c45_ids(), mdio_c45_ids is
> passed from of_mdio.c to phy_device.c as an external variable.
> In get_phy_device a copy of the mdio_c45_ids is done over the
> local c45_ids (wich are empty). After the copying, the c45_ids
> will also contain the static device found from dev-addr.
> Having dev-addr stored in devices_addrs, in get_phy_c45_ids(),
> when probing the identifiers, dev-addr can be extracted from
> devices_addrs and probed if devices_addrs[current_identifier]
> is not 0.
> This way changing the kernel API is avoided completely.
>
> As a plus to this patch, num_ids in get_phy_c45_ids,
> has the value 8 (ARRAY_SIZE(c45_ids->device_ids)),
> but the u32 *devs can store 32 devices in the bitfield.
> If a device is stored in *devs, in bits 32 to 9, it
> will not be found. This is the reason for changing
> in phy.h, the size of device_ids array.
>
> Signed-off-by: Vicentiu Galanopulo <vicentiu.galanopulo@nxp.com>
> ---
Correct me if I am completely misunderstanding the problem, but have
you considered doing the following:
- if all you need to is to replace instances of loops that do:
if (phydev->is_c45) {
for (i = 1; i < num_ids; i++) {
if (!(phydev->c45_ids.devices_in_package & (1 <<
i)))
continue;
with one that starts at dev-addr, as specified by Device Tree, then I
suspect there is an easier way to do what you want rather than your
fairly intrusive patch to do that:
- patch of_mdiobus_register_phy() to lookup both the c45 compatible
string as well as fetch the "dev-addr" property
- provide a PHY Device Tree node that has its OUI as a compatible string
(see of_get_phy_id() for details), or if it has a specified 'dev-addr'
property, use that in lieu of the default get_phy_device() logic
- pass both to phy_device_create() and eventually introduce a helper
function that lets you populate the c45_ids structure
Then for each function that does the loop above, as long as you have a
phydev reference, you can have phydev->dev_addr = 0x1e be where to start
from, if it is 0, then start at 1 (like it currently is). If you don't
have a phy device reference, which would be get_phy_c45_ids() then just
make sure you don't call that function :)
> struct phy_c45_device_ids {
> u32 devices_in_package;
> - u32 device_ids[8];
> + u32 device_ids[32];
> + u32 devices_addrs[32];
> };
This looks like a fix in itself, so it is worth a separate patch.
--
Florian
^ permalink raw reply
* Re: [PATCH net] r8169: fix setting driver_data after register_netdev
From: Andrew Lunn @ 2018-03-26 22:40 UTC (permalink / raw)
To: Francois Romieu
Cc: Heiner Kallweit, Realtek linux nic maintainers, David Miller,
netdev@vger.kernel.org
In-Reply-To: <20180326221840.GA32458@electric-eye.fr.zoreil.com>
On Tue, Mar 27, 2018 at 12:18:40AM +0200, Francois Romieu wrote:
> Andrew Lunn <andrew@lunn.ch> :
> [...]
> > How about rtl8169_get_wol() and rtl8169_set_wol(). And
> > rtl8169_get_ethtool_stats().
>
> rtl8169_get_wol does not depend on dev->driver_data. Neither does
> rtl8169_set_wol() nor rtl8169_get_ethtool_stats().
I don't know runtime pm very well, but these functions call
pm_runtime_get_noresume and pm_runtime_put_noidle. If they can result
in calls to any of the rtl8169_runtime_* functions, pci_get_drvdata()
is going to get called.
Andrew
^ permalink raw reply
* Re: [PATCH bpf-next] bpf, tracing: unbreak lttng
From: Mathieu Desnoyers @ 2018-03-26 22:39 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: rostedt, Alexei Starovoitov, David S. Miller, Daniel Borkmann,
Linus Torvalds, Peter Zijlstra, netdev, kernel-team, linux-api
In-Reply-To: <24d0ff40-c6fd-6349-4a89-dffda22cb596@fb.com>
----- On Mar 26, 2018, at 6:25 PM, Alexei Starovoitov ast@fb.com wrote:
> On 3/26/18 3:15 PM, Steven Rostedt wrote:
>> On Mon, 26 Mar 2018 15:08:45 -0700
>> Alexei Starovoitov <ast@kernel.org> wrote:
>>
>>> for_each_kernel_tracepoint() is used by out-of-tree lttng module
>>> and therefore cannot be changed.
>>> Instead introduce kernel_tracepoint_find_by_name() to find
>>> tracepoint by name.
>>>
>>> Fixes: 9e9afbae6514 ("tracepoint: compute num_args at build time")
>>> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
>>
>> I'm curious, why can't you rebase? The first patch was never acked.
>
> because I think it makes sense to keep such things in the commit log
> and in the separate diff, so next developer is aware of what kind of
> minefield the tracpoints are.
> No wonder some maintainers refuse to add them.
Since when has it become accepted to push commits into maintainer's
subsystems without their acknowledgment first ?
The minefield you are currently walking through appears to be of your
own making, so please just rework your initial patch before it reaches
upstream.
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* Re: [PATCH bpf-next] bpf, tracing: unbreak lttng
From: Alexei Starovoitov @ 2018-03-26 22:35 UTC (permalink / raw)
To: Mathieu Desnoyers, Alexei Starovoitov
Cc: David S. Miller, Daniel Borkmann, Linus Torvalds, Peter Zijlstra,
rostedt, netdev, kernel-team, linux-api
In-Reply-To: <1523827268.612.1522103407744.JavaMail.zimbra@efficios.com>
On 3/26/18 3:30 PM, Mathieu Desnoyers wrote:
> ----- On Mar 26, 2018, at 6:08 PM, Alexei Starovoitov ast@kernel.org wrote:
> [...]
>>
>> #ifdef CONFIG_TRACEPOINTS
>> -void *
>> -for_each_kernel_tracepoint(void *(*fct)(struct tracepoint *tp, void *priv),
>> +void
>> +for_each_kernel_tracepoint(void (*fct)(struct tracepoint *tp, void *priv),
>> void *priv);
>> +struct tracepoint *kernel_tracepoint_find_by_name(const char *name);
>> #else
>> -static inline void *
>> -for_each_kernel_tracepoint(void *(*fct)(struct tracepoint *tp, void *priv),
>> +static inline void
>> +for_each_kernel_tracepoint(void (*fct)(struct tracepoint *tp, void *priv),
>> void *priv)
>> {
>> return NULL;
>> }
>
> This patch is not reverting to the old code properly. It introduces a
> static inline void function that returns NULL. Please compile-test
> with CONFIG_TRACEPOINTS=n before submitting a patch involving tracepoints.
right. good catch. v2 is coming.
^ permalink raw reply
* Re: [PATCH v2 iproute2-next 3/6] rdma: Add CM_ID resource tracking information
From: Jason Gunthorpe @ 2018-03-26 22:30 UTC (permalink / raw)
To: Steve Wise; +Cc: David Ahern, leon, stephen, netdev, linux-rdma
In-Reply-To: <b5f651c2-8f71-bc1b-32b8-f4c5175fb4a0@opengridcomputing.com>
On Mon, Mar 26, 2018 at 04:34:44PM -0500, Steve Wise wrote:
>
> On 3/26/2018 4:15 PM, Jason Gunthorpe wrote:
> > On Mon, Mar 26, 2018 at 09:30:41AM -0500, Steve Wise wrote:
> >>
> >> On 3/26/2018 9:17 AM, David Ahern wrote:
> >>> On 2/27/18 9:07 AM, Steve Wise wrote:
> >>>> diff --git a/rdma/rdma.h b/rdma/rdma.h
> >>>> index 5809f70..e55205b 100644
> >>>> +++ b/rdma/rdma.h
> >>>> @@ -18,10 +18,12 @@
> >>>> #include <libmnl/libmnl.h>
> >>>> #include <rdma/rdma_netlink.h>
> >>>> #include <time.h>
> >>>> +#include <net/if_arp.h>
> >>>>
> >>>> #include "list.h"
> >>>> #include "utils.h"
> >>>> #include "json_writer.h"
> >>>> +#include <rdma/rdma_cma.h>
> >>>>
> >>> did you forget to add rdma_cma.h? I don't see that file in my repo.
> >> It is provided by the rdma-core package, upon which rdma tool now
> >> depends for the rdma_port_space enum.
> > It is a kernel bug that enum is not in an include/uapi/rdma header
> >
> > Fix it there and don't try to use rdma-core headers to get kernel ABI.
> >
> > Jason
>
> I wish you'd commented on this just a little sooner. I just resent v3
> of this series... with rdma_cma.h included. :)
>
> How about the restrack/nldev code just translates the port space from
> enum rdma_port_space to a new ABI enum, say nldev_rdma_port_space, that
> i add to rdma_netlink.h? I'd hate to open the can of worms of trying to
> split rdma_cma.h into uabi and no uabi headers. :(
If port space is already part of the ABI there isn't much reason to
translate it.
You just need to pick the right header to put it in, since it is a verbs
define it doesn't belong in the netlink header.
Jason
^ permalink raw reply
* Re: [PATCH bpf-next] bpf, tracing: unbreak lttng
From: Mathieu Desnoyers @ 2018-03-26 22:30 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: David S. Miller, Daniel Borkmann, Linus Torvalds, Peter Zijlstra,
rostedt, netdev, kernel-team, linux-api
In-Reply-To: <20180326220845.678423-1-ast@kernel.org>
----- On Mar 26, 2018, at 6:08 PM, Alexei Starovoitov ast@kernel.org wrote:
[...]
>
> #ifdef CONFIG_TRACEPOINTS
> -void *
> -for_each_kernel_tracepoint(void *(*fct)(struct tracepoint *tp, void *priv),
> +void
> +for_each_kernel_tracepoint(void (*fct)(struct tracepoint *tp, void *priv),
> void *priv);
> +struct tracepoint *kernel_tracepoint_find_by_name(const char *name);
> #else
> -static inline void *
> -for_each_kernel_tracepoint(void *(*fct)(struct tracepoint *tp, void *priv),
> +static inline void
> +for_each_kernel_tracepoint(void (*fct)(struct tracepoint *tp, void *priv),
> void *priv)
> {
> return NULL;
> }
This patch is not reverting to the old code properly. It introduces a
static inline void function that returns NULL. Please compile-test
with CONFIG_TRACEPOINTS=n before submitting a patch involving tracepoints.
But this patch should not even be needed in the first place, because it
partially reverts changes that were introduced in the bpf-next tree without
any Acked-by from the tracing maintainers. I don't see any need to obfuscate
the git log of tracepoint.{c,h}.
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* Re: [PATCH net-next 0/2] net: broadcom: Adaptive interrupt coalescing
From: Florian Fainelli @ 2018-03-26 22:29 UTC (permalink / raw)
To: Tal Gilboa, netdev
Cc: davem, jaedon.shin, pgynther, opendmb, michael.chan, gospo,
saeedm
In-Reply-To: <4625ee80-9588-e39c-5add-3c57432c1141@gmail.com>
On 03/26/2018 03:04 PM, Florian Fainelli wrote:
> On 03/26/2018 02:16 PM, Tal Gilboa wrote:
>> On 3/23/2018 4:19 AM, Florian Fainelli wrote:
>>> Hi all,
>>>
>>> This patch series adds adaptive interrupt coalescing for the Gigabit
>>> Ethernet
>>> drivers SYSTEMPORT and GENET.
>>>
>>> This really helps lower the interrupt count and system load, as
>>> measured by
>>> vmstat for a Gigabit TCP RX session:
>>
>> I don't see an improvement in system load, the opposite - 42% vs. 100%
>> for SYSTEMPORT and 85% vs. 100% for GENET. Both with the same bandwidth.
>
> Looks like I did not extract the correct data the load could spike in
> both cases (with and without net_dim) up to 100, but averaged over the
> transmission I see the following:
>
> GENET without:
> 1 0 0 1169568 0 25556 0 0 0 0 130079 62795 2
> 86 13 0 0
>
> GENET with:
> 1 0 0 1169536 0 25556 0 0 0 0 10566 10869 1
> 21 78 0 0
>
>> Am I missing something? Talking about bandwidth, I would expect 941Mb/s
>> (assuming this is TCP over IPv4). Do you know why the reduced interrupt
>> rate doesn't improve bandwidth?
>
> I am assuming that this comes down to a latency, still capturing some
> pcap files to analyze the TCP session with wireshark and see if that is
> indeed what is going on. The test machine is actually not that great
>
>> Also, any effect on the client side (you
>> mentioned enabling TX moderation for SYSTEMPORT)?
>
> Yes, on SYSTEMPORT, being the TCP IPv4 client, I have the following:
>
> SYSTEMPORT without:
> 2 0 0 191428 0 25748 0 0 0 0 86254 264 0 41
> 59 0 0
>
> SYSTEMPORT with:
> 3 0 0 190176 0 25748 0 0 0 0 45485 31332 0
> 100 0 0 0
>
> I don't get top to agree with these load results though but it looks
> like we just have the CPU spinning more, does not look like a win.
The problem appears to be the timeout selection on TX, ignoring it
completely allows us to keep the load average down while maintaining the
bandwidth. Looks like NAPI on TX already does a good job, so interrupt
mitigation on TX is not such a great idea actually...
Also, doing UDP TX tests shows that we can lower the interrupt count by
setting an appropriate tx-frames (as expected), but we won't be lowering
the CPU load since that is inherently a CPU intensive work. Past
tx-frames=64, the bandwidth completely drops because that would be 1/2
of the ring size.
--
Florian
^ permalink raw reply
* Re: [PATCH bpf-next] bpf, tracing: unbreak lttng
From: Alexei Starovoitov @ 2018-03-26 22:25 UTC (permalink / raw)
To: Steven Rostedt, Alexei Starovoitov
Cc: davem, daniel, torvalds, peterz, mathieu.desnoyers, netdev,
kernel-team, linux-api
In-Reply-To: <20180326181532.587e9e2b@gandalf.local.home>
On 3/26/18 3:15 PM, Steven Rostedt wrote:
> On Mon, 26 Mar 2018 15:08:45 -0700
> Alexei Starovoitov <ast@kernel.org> wrote:
>
>> for_each_kernel_tracepoint() is used by out-of-tree lttng module
>> and therefore cannot be changed.
>> Instead introduce kernel_tracepoint_find_by_name() to find
>> tracepoint by name.
>>
>> Fixes: 9e9afbae6514 ("tracepoint: compute num_args at build time")
>> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
>
> I'm curious, why can't you rebase? The first patch was never acked.
because I think it makes sense to keep such things in the commit log
and in the separate diff, so next developer is aware of what kind of
minefield the tracpoints are.
No wonder some maintainers refuse to add them.
^ permalink raw reply
* Re: [PATCH net-next 4/8] dt-bindings: net: add DT bindings for Microsemi Ocelot Switch
From: Rob Herring @ 2018-03-26 22:25 UTC (permalink / raw)
To: Alexandre Belloni
Cc: David S . Miller, Allan Nielsen, razvan.stefanescu, po.liu,
Thomas Petazzoni, Andrew Lunn, Florian Fainelli, netdev,
devicetree, linux-kernel, linux-mips
In-Reply-To: <20180323201117.8416-5-alexandre.belloni@bootlin.com>
On Fri, Mar 23, 2018 at 09:11:13PM +0100, Alexandre Belloni wrote:
> DT bindings for the Ethernet switch found on Microsemi Ocelot platforms.
>
> Cc: Rob Herring <robh+dt@kernel.org>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> ---
> .../devicetree/bindings/net/mscc-ocelot.txt | 62 ++++++++++++++++++++++
> 1 file changed, 62 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/mscc-ocelot.txt
>
> diff --git a/Documentation/devicetree/bindings/net/mscc-ocelot.txt b/Documentation/devicetree/bindings/net/mscc-ocelot.txt
> new file mode 100644
> index 000000000000..ee092a85b5a0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/mscc-ocelot.txt
> @@ -0,0 +1,62 @@
> +Microsemi Ocelot network Switch
> +===============================
> +
> +The Microsemi Ocelot network switch can be found on Microsemi SoCs (VSC7513,
> +VSC7514)
What's the difference in these SoCs? You should probably have the
part#'s in the compatible string.
> +
> +Required properties:
> +- compatible: Should be "mscc,ocelot-switch"
> +- reg: Must contain an (offset, length) pair of the register set for each
> + entry in reg-names.
> +- reg-names: Must include the following entries:
> + - "sys"
> + - "rew"
> + - "qs"
> + - "hsio"
> + - "qsys"
> + - "ana"
> + - "portX" with X from 0 to the number of last port index available on that
> + switch
> +- interrupts: Should contain the switch interrupts for frame extraction and
> + frame injection
> +- interrupt-names: should contain the interrupt names: "xtr", "inj"
> +
> +Example:
> +
> + switch@1010000 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + compatible = "mscc,ocelot-switch";
> + reg = <0x1010000 0x10000>,
> + <0x1030000 0x10000>,
> + <0x1080000 0x100>,
> + <0x10d0000 0x10000>,
> + <0x11e0000 0x100>,
> + <0x11f0000 0x100>,
> + <0x1200000 0x100>,
> + <0x1210000 0x100>,
> + <0x1220000 0x100>,
> + <0x1230000 0x100>,
> + <0x1240000 0x100>,
> + <0x1250000 0x100>,
> + <0x1260000 0x100>,
> + <0x1270000 0x100>,
> + <0x1280000 0x100>,
> + <0x1800000 0x80000>,
> + <0x1880000 0x10000>;
> + reg-names = "sys", "rew", "qs", "hsio", "port0",
> + "port1", "port2", "port3", "port4", "port5",
> + "port6", "port7", "port8", "port9", "port10",
> + "qsys", "ana";
> + interrupts = <21 22>;
> + interrupt-names = "xtr", "inj";
> +
> + port0: port@0 {
> + reg = <0>;
> + phy-handle = <&phy0>;
> + };
> + port1: port@1 {
> + reg = <1>;
> + phy-handle = <&phy1>;
> + };
> + };
> --
> 2.16.2
>
^ permalink raw reply
* Re: [PATCH net-next 4/8] dt-bindings: net: add DT bindings for Microsemi Ocelot Switch
From: Rob Herring @ 2018-03-26 22:25 UTC (permalink / raw)
To: Florian Fainelli
Cc: Alexandre Belloni, David S . Miller, Allan Nielsen,
razvan.stefanescu, po.liu, Thomas Petazzoni, Andrew Lunn, netdev,
devicetree, linux-kernel, linux-mips
In-Reply-To: <a5db6109-c5d9-f573-893c-f7d66c3168c2@gmail.com>
On Fri, Mar 23, 2018 at 02:11:35PM -0700, Florian Fainelli wrote:
> On 03/23/2018 01:11 PM, Alexandre Belloni wrote:
> > DT bindings for the Ethernet switch found on Microsemi Ocelot platforms.
> >
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> > ---
> > .../devicetree/bindings/net/mscc-ocelot.txt | 62 ++++++++++++++++++++++
> > 1 file changed, 62 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/net/mscc-ocelot.txt
> >
> > diff --git a/Documentation/devicetree/bindings/net/mscc-ocelot.txt b/Documentation/devicetree/bindings/net/mscc-ocelot.txt
> > new file mode 100644
> > index 000000000000..ee092a85b5a0
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/mscc-ocelot.txt
> > @@ -0,0 +1,62 @@
> > +Microsemi Ocelot network Switch
> > +===============================
> > +
> > +The Microsemi Ocelot network switch can be found on Microsemi SoCs (VSC7513,
> > +VSC7514)
> > +
> > +Required properties:
> > +- compatible: Should be "mscc,ocelot-switch"
> > +- reg: Must contain an (offset, length) pair of the register set for each
> > + entry in reg-names.
> > +- reg-names: Must include the following entries:
> > + - "sys"
> > + - "rew"
> > + - "qs"
> > + - "hsio"
> > + - "qsys"
> > + - "ana"
> > + - "portX" with X from 0 to the number of last port index available on that
> > + switch
> > +- interrupts: Should contain the switch interrupts for frame extraction and
> > + frame injection
> > +- interrupt-names: should contain the interrupt names: "xtr", "inj"
>
> You are not documenting the "ports" subnode(s).Please move the
> individual ports definition under a ports subnode, mainly for two reasons:
>
> - it makes it easy at the .dtsi level to have all ports disabled by default
>
> - this makes you strictly conforming to the DSA binding for Ethernet
> switches and this is good for consistency (both parsing code and just
> representation).
ports and port collide with the OF graph binding. It would be good if
this moved to ethernet-port(s) or similar.
Rob
^ permalink raw reply
* Re: [RFC PATCH v2] net: phy: Added device tree binding for dev-addr and dev-addr code check-up
From: Rob Herring @ 2018-03-26 22:25 UTC (permalink / raw)
To: Vicentiu Galanopulo
Cc: netdev, linux-kernel, mark.rutland, davem, marcel, devicetree,
madalin.bucur, alexandru.marginean
In-Reply-To: <20180323150522.9603-1-vicentiu.galanopulo@nxp.com>
On Fri, Mar 23, 2018 at 10:05:22AM -0500, Vicentiu Galanopulo wrote:
> Reason for this patch is that the Inphi PHY has a
> vendor specific address space for accessing the
> C45 MDIO registers - starting from 0x1e.
>
> A search of the dev-addr property is done in of_mdiobus_register.
> If the property is found in the PHY node,
> of_mdiobus_register_static_phy is called. This is a
> wrapper function for of_mdiobus_register_phy which finds the
> device in package based on dev-addr and fills devices_addrs:
> devices_addrs is a new field added to phy_c45_device_ids.
> This new field will store the dev-addr property on the same
> index where the device in package has been found.
> In order to have dev-addr in get_phy_c45_ids(), mdio_c45_ids is
> passed from of_mdio.c to phy_device.c as an external variable.
> In get_phy_device a copy of the mdio_c45_ids is done over the
> local c45_ids (wich are empty). After the copying, the c45_ids
> will also contain the static device found from dev-addr.
> Having dev-addr stored in devices_addrs, in get_phy_c45_ids(),
> when probing the identifiers, dev-addr can be extracted from
> devices_addrs and probed if devices_addrs[current_identifier]
> is not 0.
> This way changing the kernel API is avoided completely.
>
> As a plus to this patch, num_ids in get_phy_c45_ids,
> has the value 8 (ARRAY_SIZE(c45_ids->device_ids)),
> but the u32 *devs can store 32 devices in the bitfield.
> If a device is stored in *devs, in bits 32 to 9, it
> will not be found. This is the reason for changing
> in phy.h, the size of device_ids array.
>
> Signed-off-by: Vicentiu Galanopulo <vicentiu.galanopulo@nxp.com>
> ---
> Documentation/devicetree/bindings/net/phy.txt | 6 ++
Please split bindings to separate patch.
> drivers/net/phy/phy_device.c | 22 +++++-
> drivers/of/of_mdio.c | 98 ++++++++++++++++++++++++++-
> include/linux/phy.h | 5 +-
> 4 files changed, 125 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/phy.txt b/Documentation/devicetree/bindings/net/phy.txt
> index d2169a5..82692e2 100644
> --- a/Documentation/devicetree/bindings/net/phy.txt
> +++ b/Documentation/devicetree/bindings/net/phy.txt
> @@ -61,6 +61,11 @@ Optional Properties:
> - reset-deassert-us: Delay after the reset was deasserted in microseconds.
> If this property is missing the delay will be skipped.
>
> +- dev-addr: If set, it indicates the device address of the PHY to be used
> + when accessing the C45 PHY registers over MDIO. It is used for vendor specific
> + register space addresses that do no conform to standard address for the MDIO
> + registers (e.g. MMD30)
This is a 2nd MDIO address, right? Can't you just append this to reg
property?
Rob
^ permalink raw reply
* Re: [PATCH net] r8169: fix setting driver_data after register_netdev
From: Francois Romieu @ 2018-03-26 22:18 UTC (permalink / raw)
To: Andrew Lunn
Cc: Heiner Kallweit, Realtek linux nic maintainers, David Miller,
netdev@vger.kernel.org
In-Reply-To: <20180325232439.GD19365@lunn.ch>
Andrew Lunn <andrew@lunn.ch> :
[...]
> How about rtl8169_get_wol() and rtl8169_set_wol(). And
> rtl8169_get_ethtool_stats().
rtl8169_get_wol does not depend on dev->driver_data. Neither does
rtl8169_set_wol() nor rtl8169_get_ethtool_stats().
> Basically anything which makes use of run time power management
> could be invoked as soon as parts of register_netdev() have been
> called.
Ok, it can crash through rtl_open and check_link_status.
If rtl_open can be called that early, rtl_init_one::rtl8168_driver_start()
may also be executed a bit late.
--
Ueimor
^ permalink raw reply
* Re: [PATCH net-next 04/12] dt-bindings: net: dwmac-sun8i: Sort syscon compatibles by alphabetical order
From: Rob Herring @ 2018-03-26 22:23 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Maxime Ripard, Michael Turquette, Stephen Boyd,
Giuseppe Cavallaro, Mark Rutland, Mark Brown, linux-arm-kernel,
linux-clk, devicetree, netdev, Corentin Labbe, Icenowy Zheng
In-Reply-To: <20180317092857.4396-5-wens@csie.org>
On Sat, Mar 17, 2018 at 05:28:49PM +0800, Chen-Yu Tsai wrote:
> The A83T syscon compatible was appended to the syscon compatibles list,
> instead of inserted in to preserve the ordering.
>
> Move it to the proper place to keep the list sorted.
>
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
> Documentation/devicetree/bindings/net/dwmac-sun8i.txt | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH bpf-next] bpf, tracing: unbreak lttng
From: Steven Rostedt @ 2018-03-26 22:15 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: davem, daniel, torvalds, peterz, mathieu.desnoyers, netdev,
kernel-team, linux-api
In-Reply-To: <20180326220845.678423-1-ast@kernel.org>
On Mon, 26 Mar 2018 15:08:45 -0700
Alexei Starovoitov <ast@kernel.org> wrote:
> for_each_kernel_tracepoint() is used by out-of-tree lttng module
> and therefore cannot be changed.
> Instead introduce kernel_tracepoint_find_by_name() to find
> tracepoint by name.
>
> Fixes: 9e9afbae6514 ("tracepoint: compute num_args at build time")
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
I'm curious, why can't you rebase? The first patch was never acked.
-- Steve
^ permalink raw reply
* [PATCH bpf-next] bpf, tracing: unbreak lttng
From: Alexei Starovoitov @ 2018-03-26 22:08 UTC (permalink / raw)
To: davem
Cc: daniel, torvalds, peterz, rostedt, mathieu.desnoyers, netdev,
kernel-team, linux-api
for_each_kernel_tracepoint() is used by out-of-tree lttng module
and therefore cannot be changed.
Instead introduce kernel_tracepoint_find_by_name() to find
tracepoint by name.
Fixes: 9e9afbae6514 ("tracepoint: compute num_args at build time")
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
include/linux/tracepoint.h | 14 ++++++++++----
kernel/bpf/syscall.c | 11 +----------
kernel/tracepoint.c | 36 ++++++++++++++++++++----------------
3 files changed, 31 insertions(+), 30 deletions(-)
diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
index 2194e7c31484..035887dc4ed3 100644
--- a/include/linux/tracepoint.h
+++ b/include/linux/tracepoint.h
@@ -42,16 +42,22 @@ extern int
tracepoint_probe_unregister(struct tracepoint *tp, void *probe, void *data);
#ifdef CONFIG_TRACEPOINTS
-void *
-for_each_kernel_tracepoint(void *(*fct)(struct tracepoint *tp, void *priv),
+void
+for_each_kernel_tracepoint(void (*fct)(struct tracepoint *tp, void *priv),
void *priv);
+struct tracepoint *kernel_tracepoint_find_by_name(const char *name);
#else
-static inline void *
-for_each_kernel_tracepoint(void *(*fct)(struct tracepoint *tp, void *priv),
+static inline void
+for_each_kernel_tracepoint(void (*fct)(struct tracepoint *tp, void *priv),
void *priv)
{
return NULL;
}
+static inline struct tracepoint *
+kernel_tracepoint_find_by_name(const char *name)
+{
+ return NULL;
+}
#endif
#ifdef CONFIG_MODULES
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index ae8b43f1cee3..644311777d8e 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1334,15 +1334,6 @@ static const struct file_operations bpf_raw_tp_fops = {
.write = bpf_dummy_write,
};
-static void *__find_tp(struct tracepoint *tp, void *priv)
-{
- char *name = priv;
-
- if (!strcmp(tp->name, name))
- return tp;
- return NULL;
-}
-
#define BPF_RAW_TRACEPOINT_OPEN_LAST_FIELD raw_tracepoint.prog_fd
static int bpf_raw_tracepoint_open(const union bpf_attr *attr)
@@ -1358,7 +1349,7 @@ static int bpf_raw_tracepoint_open(const union bpf_attr *attr)
return -EFAULT;
tp_name[sizeof(tp_name) - 1] = 0;
- tp = for_each_kernel_tracepoint(__find_tp, tp_name);
+ tp = kernel_tracepoint_find_by_name(tp_name);
if (!tp)
return -ENOENT;
diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
index 3f2dc5738c2b..764d02fbe782 100644
--- a/kernel/tracepoint.c
+++ b/kernel/tracepoint.c
@@ -502,22 +502,17 @@ static __init int init_tracepoints(void)
__initcall(init_tracepoints);
#endif /* CONFIG_MODULES */
-static void *for_each_tracepoint_range(struct tracepoint * const *begin,
- struct tracepoint * const *end,
- void *(*fct)(struct tracepoint *tp, void *priv),
- void *priv)
+static void for_each_tracepoint_range(struct tracepoint * const *begin,
+ struct tracepoint * const *end,
+ void (*fct)(struct tracepoint *tp, void *priv),
+ void *priv)
{
struct tracepoint * const *iter;
- void *ret;
if (!begin)
- return NULL;
- for (iter = begin; iter < end; iter++) {
- ret = fct(*iter, priv);
- if (ret)
- return ret;
- }
- return NULL;
+ return;
+ for (iter = begin; iter < end; iter++)
+ fct(*iter, priv);
}
/**
@@ -525,14 +520,23 @@ static void *for_each_tracepoint_range(struct tracepoint * const *begin,
* @fct: callback
* @priv: private data
*/
-void *for_each_kernel_tracepoint(void *(*fct)(struct tracepoint *tp, void *priv),
- void *priv)
+void for_each_kernel_tracepoint(void (*fct)(struct tracepoint *tp, void *priv),
+ void *priv)
{
- return for_each_tracepoint_range(__start___tracepoints_ptrs,
- __stop___tracepoints_ptrs, fct, priv);
+ for_each_tracepoint_range(__start___tracepoints_ptrs,
+ __stop___tracepoints_ptrs, fct, priv);
}
EXPORT_SYMBOL_GPL(for_each_kernel_tracepoint);
+struct tracepoint *kernel_tracepoint_find_by_name(const char *name)
+{
+ struct tracepoint * const *tp = __start___tracepoints_ptrs;
+
+ for (; tp < __stop___tracepoints_ptrs; tp++)
+ if (!strcmp((*tp)->name, name))
+ return *tp;
+ return NULL;
+}
#ifdef CONFIG_HAVE_SYSCALL_TRACEPOINTS
/* NB: reg/unreg are called while guarded with the tracepoints_mutex */
--
2.9.5
^ permalink raw reply related
* [Patch net] llc: properly handle dev_queue_xmit() return value
From: Cong Wang @ 2018-03-26 22:08 UTC (permalink / raw)
To: netdev; +Cc: noamr, Cong Wang
llc_conn_send_pdu() pushes the skb into write queue and
calls llc_conn_send_pdus() to flush them out. However, the
status of dev_queue_xmit() is not returned to caller,
in this case, llc_conn_state_process().
llc_conn_state_process() needs hold the skb no matter
success or failure, because it still uses it after that,
therefore we should hold skb before dev_queue_xmit() when
that skb is the one being processed by llc_conn_state_process().
For other callers, they can just pass NULL and ignore
the return value as they are.
Reported-by: Noam Rathaus <noamr@beyondsecurity.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
include/net/llc_conn.h | 2 +-
net/llc/llc_c_ac.c | 15 +++++++++------
net/llc/llc_conn.c | 32 +++++++++++++++++++++++---------
3 files changed, 33 insertions(+), 16 deletions(-)
diff --git a/include/net/llc_conn.h b/include/net/llc_conn.h
index fe994d2e5286..5c40f118c0fa 100644
--- a/include/net/llc_conn.h
+++ b/include/net/llc_conn.h
@@ -103,7 +103,7 @@ void llc_sk_reset(struct sock *sk);
/* Access to a connection */
int llc_conn_state_process(struct sock *sk, struct sk_buff *skb);
-void llc_conn_send_pdu(struct sock *sk, struct sk_buff *skb);
+int llc_conn_send_pdu(struct sock *sk, struct sk_buff *skb);
void llc_conn_rtn_pdu(struct sock *sk, struct sk_buff *skb);
void llc_conn_resend_i_pdu_as_cmd(struct sock *sk, u8 nr, u8 first_p_bit);
void llc_conn_resend_i_pdu_as_rsp(struct sock *sk, u8 nr, u8 first_f_bit);
diff --git a/net/llc/llc_c_ac.c b/net/llc/llc_c_ac.c
index f59648018060..163121192aca 100644
--- a/net/llc/llc_c_ac.c
+++ b/net/llc/llc_c_ac.c
@@ -389,7 +389,7 @@ static int llc_conn_ac_send_i_cmd_p_set_0(struct sock *sk, struct sk_buff *skb)
llc_pdu_init_as_i_cmd(skb, 0, llc->vS, llc->vR);
rc = llc_mac_hdr_init(skb, llc->dev->dev_addr, llc->daddr.mac);
if (likely(!rc)) {
- llc_conn_send_pdu(sk, skb);
+ rc = llc_conn_send_pdu(sk, skb);
llc_conn_ac_inc_vs_by_1(sk, skb);
}
return rc;
@@ -916,7 +916,7 @@ static int llc_conn_ac_send_i_rsp_f_set_ackpf(struct sock *sk,
llc_pdu_init_as_i_cmd(skb, llc->ack_pf, llc->vS, llc->vR);
rc = llc_mac_hdr_init(skb, llc->dev->dev_addr, llc->daddr.mac);
if (likely(!rc)) {
- llc_conn_send_pdu(sk, skb);
+ rc = llc_conn_send_pdu(sk, skb);
llc_conn_ac_inc_vs_by_1(sk, skb);
}
return rc;
@@ -935,14 +935,17 @@ static int llc_conn_ac_send_i_rsp_f_set_ackpf(struct sock *sk,
int llc_conn_ac_send_i_as_ack(struct sock *sk, struct sk_buff *skb)
{
struct llc_sock *llc = llc_sk(sk);
+ int ret;
if (llc->ack_must_be_send) {
- llc_conn_ac_send_i_rsp_f_set_ackpf(sk, skb);
+ ret = llc_conn_ac_send_i_rsp_f_set_ackpf(sk, skb);
llc->ack_must_be_send = 0 ;
llc->ack_pf = 0;
- } else
- llc_conn_ac_send_i_cmd_p_set_0(sk, skb);
- return 0;
+ } else {
+ ret = llc_conn_ac_send_i_cmd_p_set_0(sk, skb);
+ }
+
+ return ret;
}
/**
diff --git a/net/llc/llc_conn.c b/net/llc/llc_conn.c
index 9177dbb16dce..110e32bcb399 100644
--- a/net/llc/llc_conn.c
+++ b/net/llc/llc_conn.c
@@ -30,7 +30,7 @@
#endif
static int llc_find_offset(int state, int ev_type);
-static void llc_conn_send_pdus(struct sock *sk);
+static int llc_conn_send_pdus(struct sock *sk, struct sk_buff *skb);
static int llc_conn_service(struct sock *sk, struct sk_buff *skb);
static int llc_exec_conn_trans_actions(struct sock *sk,
struct llc_conn_state_trans *trans,
@@ -193,11 +193,11 @@ int llc_conn_state_process(struct sock *sk, struct sk_buff *skb)
return rc;
}
-void llc_conn_send_pdu(struct sock *sk, struct sk_buff *skb)
+int llc_conn_send_pdu(struct sock *sk, struct sk_buff *skb)
{
/* queue PDU to send to MAC layer */
skb_queue_tail(&sk->sk_write_queue, skb);
- llc_conn_send_pdus(sk);
+ return llc_conn_send_pdus(sk, skb);
}
/**
@@ -255,7 +255,7 @@ void llc_conn_resend_i_pdu_as_cmd(struct sock *sk, u8 nr, u8 first_p_bit)
if (howmany_resend > 0)
llc->vS = (llc->vS + 1) % LLC_2_SEQ_NBR_MODULO;
/* any PDUs to re-send are queued up; start sending to MAC */
- llc_conn_send_pdus(sk);
+ llc_conn_send_pdus(sk, NULL);
out:;
}
@@ -296,7 +296,7 @@ void llc_conn_resend_i_pdu_as_rsp(struct sock *sk, u8 nr, u8 first_f_bit)
if (howmany_resend > 0)
llc->vS = (llc->vS + 1) % LLC_2_SEQ_NBR_MODULO;
/* any PDUs to re-send are queued up; start sending to MAC */
- llc_conn_send_pdus(sk);
+ llc_conn_send_pdus(sk, NULL);
out:;
}
@@ -340,12 +340,16 @@ int llc_conn_remove_acked_pdus(struct sock *sk, u8 nr, u16 *how_many_unacked)
/**
* llc_conn_send_pdus - Sends queued PDUs
* @sk: active connection
+ * @hold_skb: the skb held by caller, or NULL if does not care
*
- * Sends queued pdus to MAC layer for transmission.
+ * Sends queued pdus to MAC layer for transmission. When @hold_skb is
+ * NULL, always return 0. Otherwise, return 0 if @hold_skb is sent
+ * successfully, or 1 for failure.
*/
-static void llc_conn_send_pdus(struct sock *sk)
+static int llc_conn_send_pdus(struct sock *sk, struct sk_buff *hold_skb)
{
struct sk_buff *skb;
+ int ret = 0;
while ((skb = skb_dequeue(&sk->sk_write_queue)) != NULL) {
struct llc_pdu_sn *pdu = llc_pdu_sn_hdr(skb);
@@ -357,10 +361,20 @@ static void llc_conn_send_pdus(struct sock *sk)
skb_queue_tail(&llc_sk(sk)->pdu_unack_q, skb);
if (!skb2)
break;
- skb = skb2;
+ dev_queue_xmit(skb2);
+ } else {
+ bool is_target = skb == hold_skb;
+ int rc;
+
+ if (is_target)
+ skb_get(skb);
+ rc = dev_queue_xmit(skb);
+ if (is_target)
+ ret = rc;
}
- dev_queue_xmit(skb);
}
+
+ return ret;
}
/**
--
2.13.0
^ permalink raw reply related
* Re: [PATCH net-next 0/2] net: broadcom: Adaptive interrupt coalescing
From: Florian Fainelli @ 2018-03-26 22:04 UTC (permalink / raw)
To: Tal Gilboa, netdev
Cc: davem, jaedon.shin, pgynther, opendmb, michal.chan, gospo, saeedm
In-Reply-To: <a0263c2c-6fc6-45d4-9163-2a96b7d34492@mellanox.com>
On 03/26/2018 02:16 PM, Tal Gilboa wrote:
> On 3/23/2018 4:19 AM, Florian Fainelli wrote:
>> Hi all,
>>
>> This patch series adds adaptive interrupt coalescing for the Gigabit
>> Ethernet
>> drivers SYSTEMPORT and GENET.
>>
>> This really helps lower the interrupt count and system load, as
>> measured by
>> vmstat for a Gigabit TCP RX session:
>
> I don't see an improvement in system load, the opposite - 42% vs. 100%
> for SYSTEMPORT and 85% vs. 100% for GENET. Both with the same bandwidth.
Looks like I did not extract the correct data the load could spike in
both cases (with and without net_dim) up to 100, but averaged over the
transmission I see the following:
GENET without:
1 0 0 1169568 0 25556 0 0 0 0 130079 62795 2
86 13 0 0
GENET with:
1 0 0 1169536 0 25556 0 0 0 0 10566 10869 1
21 78 0 0
> Am I missing something? Talking about bandwidth, I would expect 941Mb/s
> (assuming this is TCP over IPv4). Do you know why the reduced interrupt
> rate doesn't improve bandwidth?
I am assuming that this comes down to a latency, still capturing some
pcap files to analyze the TCP session with wireshark and see if that is
indeed what is going on. The test machine is actually not that great
> Also, any effect on the client side (you
> mentioned enabling TX moderation for SYSTEMPORT)?
Yes, on SYSTEMPORT, being the TCP IPv4 client, I have the following:
SYSTEMPORT without:
2 0 0 191428 0 25748 0 0 0 0 86254 264 0 41
59 0 0
SYSTEMPORT with:
3 0 0 190176 0 25748 0 0 0 0 45485 31332 0
100 0 0 0
I don't get top to agree with these load results though but it looks
like we just have the CPU spinning more, does not look like a win.
Thanks a lot for taking a look at this Tal!
--
Florian
^ permalink raw reply
* Re: [RFC PATCH 00/24] Introducing AF_XDP support
From: William Tu @ 2018-03-26 21:58 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Björn Töpel, magnus.karlsson, Alexander Duyck,
Alexander Duyck, John Fastabend, Alexei Starovoitov,
willemdebruijn.kernel, Daniel Borkmann,
Linux Kernel Network Developers, Björn Töpel,
michael.lundkvist, jesse.brandeburg, anjali.singhai,
jeffrey.b.shaw, ferruh.yigit, qi.z.zhang
In-Reply-To: <20180326183810.2ef4e29f@redhat.com>
Hi Jesper,
Thanks a lot for your prompt reply.
>> Hi,
>> I also did an evaluation of AF_XDP, however the performance isn't as
>> good as above.
>> I'd like to share the result and see if there are some tuning suggestions.
>>
>> System:
>> 16 core, Intel(R) Xeon(R) CPU E5-2440 v2 @ 1.90GHz
>> Intel 10G X540-AT2 ---> so I can only run XDP_SKB mode
>
> Hmmm, why is X540-AT2 not able to use XDP natively?
Because I'm only able to use ixgbe driver for this NIC,
and AF_XDP patch only has i40e support?
>
>> AF_XDP performance:
>> Benchmark XDP_SKB
>> rxdrop 1.27 Mpps
>> txpush 0.99 Mpps
>> l2fwd 0.85 Mpps
>
> Definitely too low...
>
I did another run, the rxdrop seems better.
Benchmark XDP_SKB
rxdrop 2.3 Mpps
txpush 1.05 Mpps
l2fwd 0.90 Mpps
> What is the performance if you drop packets via iptables?
>
> Command:
> $ iptables -t raw -I PREROUTING -p udp --dport 9 --j DROP
>
I did
# iptables -t raw -I PREROUTING -p udp -i enp10s0f0 -j DROP
# iptables -nvL -t raw; sleep 10; iptables -nvL -t raw
and I got 2.9Mpps.
>> NIC configuration:
>> the command
>> "ethtool -N p3p2 flow-type udp4 src-port 4242 dst-port 4242 action 16"
>> doesn't work on my ixgbe driver, so I use ntuple:
>>
>> ethtool -K enp10s0f0 ntuple on
>> ethtool -U enp10s0f0 flow-type udp4 src-ip 10.1.1.100 action 1
>> then
>> echo 1 > /proc/sys/net/core/bpf_jit_enable
>> ./xdpsock -i enp10s0f0 -r -S --queue=1
>>
>> I also take a look at perf result:
>> For rxdrop:
>> 86.56% xdpsock xdpsock [.] main
>> 9.22% xdpsock [kernel.vmlinux] [k] nmi
>> 4.23% xdpsock xdpsock [.] xq_enq
>
> It looks very strange that you see non-maskable interrupt's (NMI) being
> this high...
>
yes, that's weird. Looking at the perf annotate of nmi,
it shows 100% spent on nop instruction.
>
>> For l2fwd:
>> 20.81% xdpsock xdpsock [.] main
>> 10.64% xdpsock [kernel.vmlinux] [k] clflush_cache_range
>
> Oh, clflush_cache_range is being called!
I though clflush_cache_range is high because we have many smp_rmb, smp_wmb
in the xdpsock queue/ring management userspace code.
(perf shows that 75% of this 10.64% spent on mfence instruction.)
> Do your system use an IOMMU ?
>
Yes.
With CONFIG_INTEL_IOMMU=y
and I saw some related functions called (ex: intel_alloc_iova).
>> 8.46% xdpsock [kernel.vmlinux] [k] xsk_sendmsg
>> 6.72% xdpsock [kernel.vmlinux] [k] skb_set_owner_w
>> 5.89% xdpsock [kernel.vmlinux] [k] __domain_mapping
>> 5.74% xdpsock [kernel.vmlinux] [k] alloc_skb_with_frags
>> 4.62% xdpsock [kernel.vmlinux] [k] netif_skb_features
>> 3.96% xdpsock [kernel.vmlinux] [k] ___slab_alloc
>> 3.18% xdpsock [kernel.vmlinux] [k] nmi
>
> Again high count for NMI ?!?
>
> Maybe you just forgot to tell perf that you want it to decode the
> bpf_prog correctly?
>
> https://prototype-kernel.readthedocs.io/en/latest/bpf/troubleshooting.html#perf-tool-symbols
>
> Enable via:
> $ sysctl net/core/bpf_jit_kallsyms=1
>
> And use perf report (while BPF is STILL LOADED):
>
> $ perf report --kallsyms=/proc/kallsyms
>
> E.g. for emailing this you can use this command:
>
> $ perf report --sort cpu,comm,dso,symbol --kallsyms=/proc/kallsyms --no-children --stdio -g none | head -n 40
>
Thanks, I followed the steps, the result of l2fwd
# Total Lost Samples: 119
#
# Samples: 2K of event 'cycles:ppp'
# Event count (approx.): 25675705627
#
# Overhead CPU Command Shared Object Symbol
# ........ ... ....... .................. ..................................
#
10.48% 013 xdpsock xdpsock [.] main
9.77% 013 xdpsock [kernel.vmlinux] [k] clflush_cache_range
8.45% 013 xdpsock [kernel.vmlinux] [k] nmi
8.07% 013 xdpsock [kernel.vmlinux] [k] xsk_sendmsg
7.81% 013 xdpsock [kernel.vmlinux] [k] __domain_mapping
4.95% 013 xdpsock [kernel.vmlinux] [k] ixgbe_xmit_frame_ring
4.66% 013 xdpsock [kernel.vmlinux] [k] skb_store_bits
4.39% 013 xdpsock [kernel.vmlinux] [k] syscall_return_via_sysret
3.93% 013 xdpsock [kernel.vmlinux] [k] pfn_to_dma_pte
2.62% 013 xdpsock [kernel.vmlinux] [k] __intel_map_single
2.53% 013 xdpsock [kernel.vmlinux] [k] __alloc_skb
2.36% 013 xdpsock [kernel.vmlinux] [k] iommu_no_mapping
2.21% 013 xdpsock [kernel.vmlinux] [k] alloc_skb_with_frags
2.07% 013 xdpsock [kernel.vmlinux] [k] skb_set_owner_w
1.98% 013 xdpsock [kernel.vmlinux] [k] __kmalloc_node_track_caller
1.94% 013 xdpsock [kernel.vmlinux] [k] ksize
1.84% 013 xdpsock [kernel.vmlinux] [k] validate_xmit_skb_list
1.62% 013 xdpsock [kernel.vmlinux] [k] kmem_cache_alloc_node
1.48% 013 xdpsock [kernel.vmlinux] [k] __kmalloc_reserve.isra.37
1.21% 013 xdpsock xdpsock [.] xq_enq
1.08% 013 xdpsock [kernel.vmlinux] [k] intel_alloc_iova
And l2fwd under "perf stat" looks OK to me. There is little context
switches, cpu
is fully utilized, 1.17 insn per cycle seems ok.
Performance counter stats for 'CPU(s) 6':
10000.787420 cpu-clock (msec) # 1.000 CPUs
utilized
24 context-switches # 0.002 K/sec
0 cpu-migrations # 0.000 K/sec
0 page-faults # 0.000 K/sec
22,361,333,647 cycles # 2.236 GHz
13,458,442,838 stalled-cycles-frontend # 60.19% frontend
cycles idle
26,251,003,067 instructions # 1.17 insn per
cycle
# 0.51 stalled
cycles per insn
4,938,921,868 branches # 493.853 M/sec
7,591,739 branch-misses # 0.15% of all
branches
10.000835769 seconds time elapsed
Will continue investigate...
Thanks
William
^ permalink raw reply
* [net-next 10/10] i40e: add support for XDP_REDIRECT
From: Jeff Kirsher @ 2018-03-26 21:41 UTC (permalink / raw)
To: davem
Cc: Björn Töpel, netdev, nhorman, sassmann, jogreene,
Jeff Kirsher
In-Reply-To: <20180326214103.18218-1-jeffrey.t.kirsher@intel.com>
From: Björn Töpel <bjorn.topel@intel.com>
The driver now acts upon the XDP_REDIRECT return action. Two new ndos
are implemented, ndo_xdp_xmit and ndo_xdp_flush.
XDP_REDIRECT action enables XDP program to redirect frames to other
netdevs.
Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e_main.c | 2 +
drivers/net/ethernet/intel/i40e/i40e_txrx.c | 74 +++++++++++++++++++++++++----
drivers/net/ethernet/intel/i40e/i40e_txrx.h | 2 +
3 files changed, 68 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 5efd6d7bfa59..16229998fb1e 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -11815,6 +11815,8 @@ static const struct net_device_ops i40e_netdev_ops = {
.ndo_bridge_getlink = i40e_ndo_bridge_getlink,
.ndo_bridge_setlink = i40e_ndo_bridge_setlink,
.ndo_bpf = i40e_xdp,
+ .ndo_xdp_xmit = i40e_xdp_xmit,
+ .ndo_xdp_flush = i40e_xdp_flush,
};
/**
diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index 9c338cef8315..f174c72480ab 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -2214,7 +2214,7 @@ static int i40e_xmit_xdp_ring(struct xdp_buff *xdp,
static struct sk_buff *i40e_run_xdp(struct i40e_ring *rx_ring,
struct xdp_buff *xdp)
{
- int result = I40E_XDP_PASS;
+ int err, result = I40E_XDP_PASS;
struct i40e_ring *xdp_ring;
struct bpf_prog *xdp_prog;
u32 act;
@@ -2233,6 +2233,10 @@ static struct sk_buff *i40e_run_xdp(struct i40e_ring *rx_ring,
xdp_ring = rx_ring->vsi->xdp_rings[rx_ring->queue_index];
result = i40e_xmit_xdp_ring(xdp, xdp_ring);
break;
+ case XDP_REDIRECT:
+ err = xdp_do_redirect(rx_ring->netdev, xdp, xdp_prog);
+ result = !err ? I40E_XDP_TX : I40E_XDP_CONSUMED;
+ break;
default:
bpf_warn_invalid_xdp_action(act);
case XDP_ABORTED:
@@ -2268,6 +2272,15 @@ static void i40e_rx_buffer_flip(struct i40e_ring *rx_ring,
#endif
}
+static inline void i40e_xdp_ring_update_tail(struct i40e_ring *xdp_ring)
+{
+ /* Force memory writes to complete before letting h/w
+ * know there are new descriptors to fetch.
+ */
+ wmb();
+ writel_relaxed(xdp_ring->next_to_use, xdp_ring->tail);
+}
+
/**
* i40e_clean_rx_irq - Clean completed descriptors from Rx ring - bounce buf
* @rx_ring: rx descriptor ring to transact packets on
@@ -2402,16 +2415,11 @@ static int i40e_clean_rx_irq(struct i40e_ring *rx_ring, int budget)
}
if (xdp_xmit) {
- struct i40e_ring *xdp_ring;
-
- xdp_ring = rx_ring->vsi->xdp_rings[rx_ring->queue_index];
+ struct i40e_ring *xdp_ring =
+ rx_ring->vsi->xdp_rings[rx_ring->queue_index];
- /* Force memory writes to complete before letting h/w
- * know there are new descriptors to fetch.
- */
- wmb();
-
- writel(xdp_ring->next_to_use, xdp_ring->tail);
+ i40e_xdp_ring_update_tail(xdp_ring);
+ xdp_do_flush_map();
}
rx_ring->skb = skb;
@@ -3659,3 +3667,49 @@ netdev_tx_t i40e_lan_xmit_frame(struct sk_buff *skb, struct net_device *netdev)
return i40e_xmit_frame_ring(skb, tx_ring);
}
+
+/**
+ * i40e_xdp_xmit - Implements ndo_xdp_xmit
+ * @dev: netdev
+ * @xdp: XDP buffer
+ *
+ * Returns Zero if sent, else an error code
+ **/
+int i40e_xdp_xmit(struct net_device *dev, struct xdp_buff *xdp)
+{
+ struct i40e_netdev_priv *np = netdev_priv(dev);
+ unsigned int queue_index = smp_processor_id();
+ struct i40e_vsi *vsi = np->vsi;
+ int err;
+
+ if (test_bit(__I40E_VSI_DOWN, vsi->state))
+ return -ENETDOWN;
+
+ if (!i40e_enabled_xdp_vsi(vsi) || queue_index >= vsi->num_queue_pairs)
+ return -ENXIO;
+
+ err = i40e_xmit_xdp_ring(xdp, vsi->xdp_rings[queue_index]);
+ if (err != I40E_XDP_TX)
+ return -ENOSPC;
+
+ return 0;
+}
+
+/**
+ * i40e_xdp_flush - Implements ndo_xdp_flush
+ * @dev: netdev
+ **/
+void i40e_xdp_flush(struct net_device *dev)
+{
+ struct i40e_netdev_priv *np = netdev_priv(dev);
+ unsigned int queue_index = smp_processor_id();
+ struct i40e_vsi *vsi = np->vsi;
+
+ if (test_bit(__I40E_VSI_DOWN, vsi->state))
+ return;
+
+ if (!i40e_enabled_xdp_vsi(vsi) || queue_index >= vsi->num_queue_pairs)
+ return;
+
+ i40e_xdp_ring_update_tail(vsi->xdp_rings[queue_index]);
+}
diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.h b/drivers/net/ethernet/intel/i40e/i40e_txrx.h
index 7f8220e65374..3043483ec426 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.h
@@ -510,6 +510,8 @@ u32 i40e_get_tx_pending(struct i40e_ring *ring, bool in_sw);
void i40e_detect_recover_hung(struct i40e_vsi *vsi);
int __i40e_maybe_stop_tx(struct i40e_ring *tx_ring, int size);
bool __i40e_chk_linearize(struct sk_buff *skb);
+int i40e_xdp_xmit(struct net_device *dev, struct xdp_buff *xdp);
+void i40e_xdp_flush(struct net_device *dev);
/**
* i40e_get_head - Retrieve head from head writeback
--
2.14.3
^ permalink raw reply related
* [net-next 07/10] i40e: stop using cmpxchg flow in i40e_set_priv_flags()
From: Jeff Kirsher @ 2018-03-26 21:41 UTC (permalink / raw)
To: davem; +Cc: Jacob Keller, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <20180326214103.18218-1-jeffrey.t.kirsher@intel.com>
From: Jacob Keller <jacob.e.keller@intel.com>
Now that the only places which modify flags are either (a) during
initialization prior to creating a netdevice, or (b) while holding the
rtnl lock, we no longer need the cmpxchg64 call in i40e_set_priv_flags.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 19 +++++--------------
1 file changed, 5 insertions(+), 14 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
index 2a9c93091e3d..b974482ff630 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
@@ -4436,21 +4436,12 @@ static int i40e_set_priv_flags(struct net_device *dev, u32 flags)
}
}
- /* Compare and exchange the new flags into place. If we failed, that
- * is if cmpxchg returns anything but the old value, this means that
- * something else has modified the flags variable since we copied it
- * originally. We'll just punt with an error and log something in the
- * message buffer.
- *
- * This is the point of no return for this function. We need to have
- * checked any discrepancies or misconfigurations and returned
- * EOPNOTSUPP before updating pf->flags here.
+ /* Now that we've checked to ensure that the new flags are valid, load
+ * them into place. Since we only modify flags either (a) during
+ * initialization or (b) while holding the RTNL lock, we don't need
+ * anything fancy here.
*/
- if (cmpxchg64(&pf->flags, orig_flags, new_flags) != orig_flags) {
- dev_warn(&pf->pdev->dev,
- "Unable to update pf->flags as it was modified by another thread...\n");
- return -EAGAIN;
- }
+ pf->flags = new_flags;
/* Process any additional changes needed as a result of flag changes.
* The changed_flags value reflects the list of bits that were
--
2.14.3
^ permalink raw reply related
* [net-next 08/10] i40e: re-number feature flags to remove gaps
From: Jeff Kirsher @ 2018-03-26 21:41 UTC (permalink / raw)
To: davem; +Cc: Jacob Keller, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <20180326214103.18218-1-jeffrey.t.kirsher@intel.com>
From: Jacob Keller <jacob.e.keller@intel.com>
Remove the gaps created by the recent refactor of various feature flags
that have moved to the state field. Use only a u32 now that we have
fewer than 32 flags in the field.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e.h | 57 ++++++++++++++++------------------
1 file changed, 26 insertions(+), 31 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
index 67518013ca4d..a44139c1de80 100644
--- a/drivers/net/ethernet/intel/i40e/i40e.h
+++ b/drivers/net/ethernet/intel/i40e/i40e.h
@@ -518,37 +518,32 @@ struct i40e_pf {
#define I40E_HW_RESTART_AUTONEG BIT(18)
#define I40E_HW_STOPPABLE_FW_LLDP BIT(19)
- u64 flags;
-#define I40E_FLAG_RX_CSUM_ENABLED BIT_ULL(0)
-#define I40E_FLAG_MSI_ENABLED BIT_ULL(1)
-#define I40E_FLAG_MSIX_ENABLED BIT_ULL(2)
-#define I40E_FLAG_RSS_ENABLED BIT_ULL(3)
-#define I40E_FLAG_VMDQ_ENABLED BIT_ULL(4)
-/* Gap for BIT_ULL(5) */
-#define I40E_FLAG_SRIOV_ENABLED BIT_ULL(6)
-#define I40E_FLAG_DCB_CAPABLE BIT_ULL(7)
-#define I40E_FLAG_DCB_ENABLED BIT_ULL(8)
-#define I40E_FLAG_FD_SB_ENABLED BIT_ULL(9)
-#define I40E_FLAG_FD_ATR_ENABLED BIT_ULL(10)
-/* Gap for BIT_ULL(11) and BIT_ULL(12) */
-#define I40E_FLAG_MFP_ENABLED BIT_ULL(13)
-/* Gap for BIT_ULL(14) */
-#define I40E_FLAG_HW_ATR_EVICT_ENABLED BIT_ULL(15)
-#define I40E_FLAG_VEB_MODE_ENABLED BIT_ULL(16)
-#define I40E_FLAG_VEB_STATS_ENABLED BIT_ULL(17)
-#define I40E_FLAG_LINK_POLLING_ENABLED BIT_ULL(18)
-#define I40E_FLAG_TRUE_PROMISC_SUPPORT BIT_ULL(19)
-/* Gap for BIT_ULL(20) */
-#define I40E_FLAG_LEGACY_RX BIT_ULL(21)
-#define I40E_FLAG_PTP BIT_ULL(22)
-#define I40E_FLAG_IWARP_ENABLED BIT_ULL(23)
-/* Gap for BIT_ULL(24) through BIT_ULL(26) */
-#define I40E_FLAG_LINK_DOWN_ON_CLOSE_ENABLED BIT_ULL(27)
-#define I40E_FLAG_SOURCE_PRUNING_DISABLED BIT_ULL(28)
-#define I40E_FLAG_TC_MQPRIO BIT_ULL(29)
-#define I40E_FLAG_FD_SB_INACTIVE BIT_ULL(30)
-#define I40E_FLAG_FD_SB_TO_CLOUD_FILTER BIT_ULL(31)
-#define I40E_FLAG_DISABLE_FW_LLDP BIT_ULL(32)
+ u32 flags;
+#define I40E_FLAG_RX_CSUM_ENABLED BIT(0)
+#define I40E_FLAG_MSI_ENABLED BIT(1)
+#define I40E_FLAG_MSIX_ENABLED BIT(2)
+#define I40E_FLAG_RSS_ENABLED BIT(3)
+#define I40E_FLAG_VMDQ_ENABLED BIT(4)
+#define I40E_FLAG_SRIOV_ENABLED BIT(5)
+#define I40E_FLAG_DCB_CAPABLE BIT(6)
+#define I40E_FLAG_DCB_ENABLED BIT(7)
+#define I40E_FLAG_FD_SB_ENABLED BIT(8)
+#define I40E_FLAG_FD_ATR_ENABLED BIT(9)
+#define I40E_FLAG_MFP_ENABLED BIT(10)
+#define I40E_FLAG_HW_ATR_EVICT_ENABLED BIT(11)
+#define I40E_FLAG_VEB_MODE_ENABLED BIT(12)
+#define I40E_FLAG_VEB_STATS_ENABLED BIT(13)
+#define I40E_FLAG_LINK_POLLING_ENABLED BIT(14)
+#define I40E_FLAG_TRUE_PROMISC_SUPPORT BIT(15)
+#define I40E_FLAG_LEGACY_RX BIT(16)
+#define I40E_FLAG_PTP BIT(17)
+#define I40E_FLAG_IWARP_ENABLED BIT(18)
+#define I40E_FLAG_LINK_DOWN_ON_CLOSE_ENABLED BIT(19)
+#define I40E_FLAG_SOURCE_PRUNING_DISABLED BIT(20)
+#define I40E_FLAG_TC_MQPRIO BIT(21)
+#define I40E_FLAG_FD_SB_INACTIVE BIT(22)
+#define I40E_FLAG_FD_SB_TO_CLOUD_FILTER BIT(23)
+#define I40E_FLAG_DISABLE_FW_LLDP BIT(24)
struct i40e_client_instance *cinst;
bool stat_offsets_loaded;
--
2.14.3
^ permalink raw reply related
* [net-next 09/10] i40e: tweak page counting for XDP_REDIRECT
From: Jeff Kirsher @ 2018-03-26 21:41 UTC (permalink / raw)
To: davem
Cc: Björn Töpel, netdev, nhorman, sassmann, jogreene,
Jeff Kirsher
In-Reply-To: <20180326214103.18218-1-jeffrey.t.kirsher@intel.com>
From: Björn Töpel <bjorn.topel@intel.com>
This commit tweaks the page counting for XDP_REDIRECT to function
properly. XDP_REDIRECT support will be added in a future commit.
The current page counting scheme assumes that the reference count
cannot decrease until the received frame is sent to the upper layers
of the networking stack. This assumption does not hold for the
XDP_REDIRECT action, since a page (pointed out by xdp_buff) can have
its reference count decreased via the xdp_do_redirect call.
To work around that, we now start off by a large page count and then
don't allow a refcount less than two.
Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e_txrx.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index 797bcdd3504e..9c338cef8315 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -1588,9 +1588,8 @@ static bool i40e_alloc_mapped_page(struct i40e_ring *rx_ring,
bi->dma = dma;
bi->page = page;
bi->page_offset = i40e_rx_offset(rx_ring);
-
- /* initialize pagecnt_bias to 1 representing we fully own page */
- bi->pagecnt_bias = 1;
+ page_ref_add(page, USHRT_MAX - 1);
+ bi->pagecnt_bias = USHRT_MAX;
return true;
}
@@ -1956,8 +1955,8 @@ static bool i40e_can_reuse_rx_page(struct i40e_rx_buffer *rx_buffer)
* the pagecnt_bias and page count so that we fully restock the
* number of references the driver holds.
*/
- if (unlikely(!pagecnt_bias)) {
- page_ref_add(page, USHRT_MAX);
+ if (unlikely(pagecnt_bias == 1)) {
+ page_ref_add(page, USHRT_MAX - 1);
rx_buffer->pagecnt_bias = USHRT_MAX;
}
--
2.14.3
^ permalink raw reply related
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