* Re: [PATCH net-next v6 06/15] ethtool: netlink bitset handling
From: Michal Kubecek @ 2019-07-04 12:53 UTC (permalink / raw)
To: netdev
Cc: Johannes Berg, Jiri Pirko, David Miller, Jakub Kicinski,
Andrew Lunn, Florian Fainelli, John Linville, Stephen Hemminger,
linux-kernel
In-Reply-To: <2f1a8edb0b000b4eb7adcaca0d1fb05fdd73a587.camel@sipsolutions.net>
On Thu, Jul 04, 2019 at 02:21:52PM +0200, Johannes Berg wrote:
> On Thu, 2019-07-04 at 14:17 +0200, Michal Kubecek wrote:
> > On Thu, Jul 04, 2019 at 02:03:02PM +0200, Johannes Berg wrote:
> > > On Thu, 2019-07-04 at 13:52 +0200, Michal Kubecek wrote:
> > > >
> > > > There is still the question if it it should be implemented as a nested
> > > > attribute which could look like the current compact form without the
> > > > "list" flag (if there is no mask, it's a list). Or an unstructured data
> > > > block consisting of u32 bit length
> > >
> > > You wouldn't really need the length, since the attribute has a length
> > > already :-)
> >
> > It has byte length, not bit length. The bitmaps we are dealing with
> > can have any bit length, not necessarily multiples of 8 (or even 32).
>
> Not sure why that matters? You have the mask, so you don't really need
> to additionally say that you're only going up to a certain bit?
>
> I mean, say you want to set some bits <=17, why would you need to say
> that they're <=17 if you have a
> value: 0b00000000'000000xx'xxxxxxxx'xxxxxxxx
> mask: 0b00000000'00000011'11111111'11111111
One scenario that I can see from the top of my head would be user
running
ethtool -s <dev> advertise 0x...
with hex value representing some subset of link modes. Now if ethtool
version is behind kernel and recognizes fewer link modes than kernel
but in a way that the number rounded up to bytes or words would be the
same, kernel has no way to recognize of those zero bits on top of the
mask are zero on purpose or just because userspace doesn't know about
them. In general, I believe the absence of bit length information is
something protocols would have to work around sometimes.
The submitted implementation doesn't have this problem as it can tell
kernel "this is a list" (i.e. I'm not sending a value/mask pair, I want
exactly these bits to be set). Thus it can easily implement requests of
both types (value/mask or just value):
ethtool -s <dev> advertise 0x2f
ethtool -s <dev> advertise 0x08/0x0c
ethtool -s <dev> advertise 100baseT/Full off 1000baseT/Full on
and could be as easily extended to support also
ethtool -s <dev> advertise 100baseT/Full 1000baseT/Full
Michal
^ permalink raw reply
* Re: [PATCH net-next 3/3] net: stmmac: Introducing support for Page Pool
From: Ilias Apalodimas @ 2019-07-04 12:49 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Jesper Dangaard Brouer, Jose Abreu, Joao Pinto, Alexandre Torgue,
Maxime Ripard, Networking, Linux Kernel Mailing List,
David S . Miller, Chen-Yu Tsai, Maxime Coquelin,
Giuseppe Cavallaro, linux-stm32, Linux ARM
In-Reply-To: <CAK8P3a3GC6f-xHG7MqZRLhY66Ui4HQVi=4WXR703wqfMNY6A5A@mail.gmail.com>
On Thu, Jul 04, 2019 at 02:14:28PM +0200, Arnd Bergmann wrote:
> On Thu, Jul 4, 2019 at 12:31 PM Ilias Apalodimas
> <ilias.apalodimas@linaro.org> wrote:
> > > On Wed, 3 Jul 2019 12:37:50 +0200
> > > Jose Abreu <Jose.Abreu@synopsys.com> wrote:
>
> > 1. page pool allocs packet. The API doesn't sync but i *think* you don't have to
> > explicitly since the CPU won't touch that buffer until the NAPI handler kicks
> > in. On the napi handler you need to dma_sync_single_for_cpu() and process the
> > packet.
>
> > So bvottom line i *think* we can skip the dma_sync_single_for_device() on the
> > initial allocation *only*. If am terribly wrong please let me know :)
>
> I think you have to do a sync_single_for_device /somewhere/ before the
> buffer is given to the device. On a non-cache-coherent machine with
> a write-back cache, there may be dirty cache lines that get written back
> after the device DMA's data into it (e.g. from a previous memset
> from before the buffer got freed), so you absolutely need to flush any
> dirty cache lines on it first.
Ok my bad here i forgot to add "when coherency is there", since the driver
i had in mind runs on such a device (i think this is configurable though so i'll
add the sync explicitly to make sure we won't break any configurations).
In general you are right, thanks for the explanation!
> You may also need to invalidate the cache lines in the following
> sync_single_for_cpu() to eliminate clean cache lines with stale data
> that got there when speculatively reading between the cache-invalidate
> and the DMA.
>
> Arnd
Thanks!
/Ilias
^ permalink raw reply
* Re: [PATCH 2/3] module: Fix up module_notifier return values.
From: Robert Richter @ 2019-07-04 12:48 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Frank Ch. Eigler, Mathieu Desnoyers, Jessica Yu, linux-kernel,
Josh Poimboeuf, jikos, mbenes, Petr Mladek, Alexei Starovoitov,
Daniel Borkmann, Andrew Morton, rostedt, Ingo Molnar,
Martin KaFai Lau, Song Liu, Yonghong Song, paulmck,
Joel Fernandes, Google, Ard Biesheuvel, Thomas Gleixner,
oprofile-list, netdev, bpf
In-Reply-To: <20190625074214.GR3436@hirez.programming.kicks-ass.net>
On 25.06.19 09:42:14, Peter Zijlstra wrote:
> On Mon, Jun 24, 2019 at 04:58:10PM -0400, Frank Ch. Eigler wrote:
> > From peterz's comments, the patches, it's not obvious to me how one is
> > to choose between 0 (NOTIFY_DONE) and 1 (NOTIFY_OK) in the case of a
> > routine success.
>
> I'm not sure either; what I think I choice was:
>
> - if I want to completely ignore the callback, use DONE (per the
> "Don't care" comment).
>
> - if we finished the notifier without error, use OK or
> notifier_from_errno(0).
>
> But yes, its a bit of a shit interface.
It looks like it was rarely used in earlier kernels as some sort of
error detection for the notifier calls, e.g.:
1da177e4c3f41524e886b7f1b8a0c1fc7321cac2:kernel/profile.c-int profile_handoff_task(struct task_struct * task)
1da177e4c3f41524e886b7f1b8a0c1fc7321cac2:kernel/profile.c-{
1da177e4c3f41524e886b7f1b8a0c1fc7321cac2:kernel/profile.c- int ret;
1da177e4c3f41524e886b7f1b8a0c1fc7321cac2:kernel/profile.c- read_lock(&handoff_lock);
1da177e4c3f41524e886b7f1b8a0c1fc7321cac2:kernel/profile.c- ret = notifier_call_chain(&task_free_notifier, 0, task);
1da177e4c3f41524e886b7f1b8a0c1fc7321cac2:kernel/profile.c- read_unlock(&handoff_lock);
1da177e4c3f41524e886b7f1b8a0c1fc7321cac2:kernel/profile.c: return (ret == NOTIFY_OK) ? 1 : 0;
1da177e4c3f41524e886b7f1b8a0c1fc7321cac2:kernel/profile.c-}
So NOTIFY_OK was used to state there is no error, while NOTIFY_DONE
says the notifier was executed and there might have been errors. The
caller may distinguish the results then.
-Robert
^ permalink raw reply
* Re: [net-next 1/3] ice: Initialize and register platform device to provide RDMA
From: Jason Gunthorpe @ 2019-07-04 12:48 UTC (permalink / raw)
To: Greg KH
Cc: Jeff Kirsher, davem@davemloft.net, dledford@redhat.com,
Tony Nguyen, netdev@vger.kernel.org, linux-rdma@vger.kernel.org,
nhorman@redhat.com, sassmann@redhat.com, poswald@suse.com,
mustafa.ismail@intel.com, shiraz.saleem@intel.com, Dave Ertman,
Andrew Bowers
In-Reply-To: <20190704124247.GA6807@kroah.com>
On Thu, Jul 04, 2019 at 02:42:47PM +0200, Greg KH wrote:
> On Thu, Jul 04, 2019 at 12:37:33PM +0000, Jason Gunthorpe wrote:
> > On Thu, Jul 04, 2019 at 02:29:50PM +0200, Greg KH wrote:
> > > On Thu, Jul 04, 2019 at 12:16:41PM +0000, Jason Gunthorpe wrote:
> > > > On Wed, Jul 03, 2019 at 07:12:50PM -0700, Jeff Kirsher wrote:
> > > > > From: Tony Nguyen <anthony.l.nguyen@intel.com>
> > > > >
> > > > > The RDMA block does not advertise on the PCI bus or any other bus.
> > > > > Thus the ice driver needs to provide access to the RDMA hardware block
> > > > > via a virtual bus; utilize the platform bus to provide this access.
> > > > >
> > > > > This patch initializes the driver to support RDMA as well as creates
> > > > > and registers a platform device for the RDMA driver to register to. At
> > > > > this point the driver is fully initialized to register a platform
> > > > > driver, however, can not yet register as the ops have not been
> > > > > implemented.
> > > >
> > > > I think you need Greg's ack on all this driver stuff - particularly
> > > > that a platform_device is OK.
> > >
> > > A platform_device is almost NEVER ok.
> > >
> > > Don't abuse it, make a real device on a real bus. If you don't have a
> > > real bus and just need to create a device to hang other things off of,
> > > then use the virtual one, that's what it is there for.
> >
> > Ideally I'd like to see all the RDMA drivers that connect to ethernet
> > drivers use some similar scheme.
>
> Why? They should be attached to a "real" device, why make any up?
? A "real" device, like struct pci_device, can only bind to one
driver. How can we bind it concurrently to net, rdma, scsi, etc?
> > This is for a PCI device that plugs into multiple subsystems in the
> > kernel, ie it has net driver functionality, rdma functionality, some
> > even have SCSI functionality
>
> Sounds like a MFD device, why aren't you using that functionality
> instead?
This was also my advice, but in another email Jeff says:
MFD architecture was also considered, and we selected the simpler
platform model. Supporting a MFD architecture would require an
additional MFD core driver, individual platform netdev, RDMA function
drivers, and stripping a large portion of the netdev drivers into
MFD core. The sub-devices registered by MFD core for function
drivers are indeed platform devices.
Thanks,
Jason
^ permalink raw reply
* Re: [PATCH net-next 8/8] net: mscc: PTP Hardware Clock (PHC) support
From: kbuild test robot @ 2019-07-04 12:48 UTC (permalink / raw)
To: Antoine Tenart
Cc: kbuild-all, davem, richardcochran, alexandre.belloni,
UNGLinuxDriver, ralf, paul.burton, jhogan, Antoine Tenart, netdev,
linux-mips, thomas.petazzoni, allan.nielsen
In-Reply-To: <20190701100327.6425-9-antoine.tenart@bootlin.com>
[-- Attachment #1: Type: text/plain, Size: 4549 bytes --]
Hi Antoine,
I love your patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
url: https://github.com/0day-ci/linux/commits/Antoine-Tenart/net-mscc-PTP-Hardware-Clock-PHC-support/20190702-182042
config: x86_64-randconfig-s2-07041950 (attached as .config)
compiler: gcc-7 (Debian 7.4.0-9) 7.4.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings
All warnings (new ones prefixed by >>):
In file included from include/linux/irqflags.h:16:0,
from arch/x86/include/asm/processor.h:33,
from arch/x86/include/asm/cpufeature.h:5,
from arch/x86/include/asm/thread_info.h:53,
from include/linux/thread_info.h:38,
from arch/x86/include/asm/preempt.h:7,
from include/linux/preempt.h:78,
from include/linux/spinlock.h:51,
from include/linux/seqlock.h:36,
from include/linux/time.h:6,
from include/linux/skbuff.h:15,
from include/linux/if_ether.h:19,
from include/linux/etherdevice.h:20,
from drivers/net/ethernet/mscc/ocelot.c:7:
drivers/net/ethernet/mscc/ocelot.c: In function 'ocelot_ptp_adjfine':
>> arch/x86/include/asm/irqflags.h:41:2: warning: 'flags' may be used uninitialized in this function [-Wmaybe-uninitialized]
asm volatile("push %0 ; popf"
^~~
drivers/net/ethernet/mscc/ocelot.c:1904:16: note: 'flags' was declared here
unsigned long flags;
^~~~~
--
In file included from include/linux/irqflags.h:16:0,
from arch/x86/include/asm/processor.h:33,
from arch/x86/include/asm/cpufeature.h:5,
from arch/x86/include/asm/thread_info.h:53,
from include/linux/thread_info.h:38,
from arch/x86/include/asm/preempt.h:7,
from include/linux/preempt.h:78,
from include/linux/spinlock.h:51,
from include/linux/seqlock.h:36,
from include/linux/time.h:6,
from include/linux/skbuff.h:15,
from include/linux/if_ether.h:19,
from include/linux/etherdevice.h:20,
from drivers/net//ethernet/mscc/ocelot.c:7:
drivers/net//ethernet/mscc/ocelot.c: In function 'ocelot_ptp_adjfine':
>> arch/x86/include/asm/irqflags.h:41:2: warning: 'flags' may be used uninitialized in this function [-Wmaybe-uninitialized]
asm volatile("push %0 ; popf"
^~~
drivers/net//ethernet/mscc/ocelot.c:1904:16: note: 'flags' was declared here
unsigned long flags;
^~~~~
vim +/flags +41 arch/x86/include/asm/irqflags.h
6abcd98f include/asm-x86/irqflags.h Glauber de Oliveira Costa 2008-01-30 37
1f59a458 arch/x86/include/asm/irqflags.h Nick Desaulniers 2018-08-27 38 extern inline void native_restore_fl(unsigned long flags);
1f59a458 arch/x86/include/asm/irqflags.h Nick Desaulniers 2018-08-27 39 extern inline void native_restore_fl(unsigned long flags)
6abcd98f include/asm-x86/irqflags.h Glauber de Oliveira Costa 2008-01-30 40 {
cf7f7191 include/asm-x86/irqflags.h Joe Perches 2008-03-23 @41 asm volatile("push %0 ; popf"
6abcd98f include/asm-x86/irqflags.h Glauber de Oliveira Costa 2008-01-30 42 : /* no output */
6abcd98f include/asm-x86/irqflags.h Glauber de Oliveira Costa 2008-01-30 43 :"g" (flags)
cf7f7191 include/asm-x86/irqflags.h Joe Perches 2008-03-23 44 :"memory", "cc");
6abcd98f include/asm-x86/irqflags.h Glauber de Oliveira Costa 2008-01-30 45 }
6abcd98f include/asm-x86/irqflags.h Glauber de Oliveira Costa 2008-01-30 46
:::::: The code at line 41 was first introduced by commit
:::::: cf7f7191cf20011e47243b594e433275a6db811b include/asm-x86/irqflags.h: checkpatch cleanups - formatting only
:::::: TO: Joe Perches <joe@perches.com>
:::::: CC: Ingo Molnar <mingo@elte.hu>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 38820 bytes --]
^ permalink raw reply
* [PATCH bpf-next v3 6/6] samples/bpf: add use of need_sleep flag in xdpsock
From: Magnus Karlsson @ 2019-07-04 12:42 UTC (permalink / raw)
To: magnus.karlsson, bjorn.topel, ast, daniel, netdev, brouer
Cc: bpf, bruce.richardson, ciara.loftus, jakub.kicinski, xiaolong.ye,
qi.z.zhang, maximmi, sridhar.samudrala, kevin.laatz,
ilias.apalodimas, kiran.patil, axboe, maciej.fijalkowski,
maciejromanfijalkowski, intel-wired-lan
In-Reply-To: <1562244134-19069-1-git-send-email-magnus.karlsson@intel.com>
This commit adds using the need_sleep flag to the xdpsock sample
application. It is turned on by default as we think it is a feature
that seems to always produce a performance benefit, if the application
has been written taking advantage of it. It can be turned off in the
sample app by using the '-m' command line option.
The txpush and l2fwd sub applications have also been updated to
support poll() with multiple sockets.
Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
samples/bpf/xdpsock_user.c | 192 ++++++++++++++++++++++++++++-----------------
1 file changed, 120 insertions(+), 72 deletions(-)
diff --git a/samples/bpf/xdpsock_user.c b/samples/bpf/xdpsock_user.c
index 93eaaf7..7d771af 100644
--- a/samples/bpf/xdpsock_user.c
+++ b/samples/bpf/xdpsock_user.c
@@ -67,8 +67,10 @@ static int opt_ifindex;
static int opt_queue;
static int opt_poll;
static int opt_interval = 1;
-static u32 opt_xdp_bind_flags;
+static u32 opt_xdp_bind_flags = XDP_USE_NEED_WAKEUP;
static int opt_xsk_frame_size = XSK_UMEM__DEFAULT_FRAME_SIZE;
+static int opt_timeout = 1000;
+static bool opt_might_sleep = true;
static __u32 prog_id;
struct xsk_umem_info {
@@ -352,6 +354,7 @@ static struct option long_options[] = {
{"zero-copy", no_argument, 0, 'z'},
{"copy", no_argument, 0, 'c'},
{"frame-size", required_argument, 0, 'f'},
+ {"no-might-sleep", no_argument, 0, 'm'},
{0, 0, 0, 0}
};
@@ -372,6 +375,7 @@ static void usage(const char *prog)
" -z, --zero-copy Force zero-copy mode.\n"
" -c, --copy Force copy mode.\n"
" -f, --frame-size=n Set the frame size (must be a power of two, default is %d).\n"
+ " -m, --no-might-sleep Turn off use of driver might sleep flag.\n"
"\n";
fprintf(stderr, str, prog, XSK_UMEM__DEFAULT_FRAME_SIZE);
exit(EXIT_FAILURE);
@@ -384,8 +388,9 @@ static void parse_command_line(int argc, char **argv)
opterr = 0;
for (;;) {
- c = getopt_long(argc, argv, "Frtli:q:psSNn:czf:", long_options,
- &option_index);
+
+ c = getopt_long(argc, argv, "Frtli:q:psSNn:czf:m",
+ long_options, &option_index);
if (c == -1)
break;
@@ -429,6 +434,9 @@ static void parse_command_line(int argc, char **argv)
break;
case 'f':
opt_xsk_frame_size = atoi(optarg);
+ case 'm':
+ opt_might_sleep = false;
+ opt_xdp_bind_flags &= ~XDP_USE_NEED_WAKEUP;
break;
default:
usage(basename(argv[0]));
@@ -459,7 +467,8 @@ static void kick_tx(struct xsk_socket_info *xsk)
exit_with_error(errno);
}
-static inline void complete_tx_l2fwd(struct xsk_socket_info *xsk)
+static inline void complete_tx_l2fwd(struct xsk_socket_info *xsk,
+ struct pollfd *fds)
{
u32 idx_cq = 0, idx_fq = 0;
unsigned int rcvd;
@@ -468,7 +477,9 @@ static inline void complete_tx_l2fwd(struct xsk_socket_info *xsk)
if (!xsk->outstanding_tx)
return;
- kick_tx(xsk);
+ if (!opt_might_sleep || xsk_ring_prod__needs_wakeup(&xsk->tx))
+ kick_tx(xsk);
+
ndescs = (xsk->outstanding_tx > BATCH_SIZE) ? BATCH_SIZE :
xsk->outstanding_tx;
@@ -482,6 +493,8 @@ static inline void complete_tx_l2fwd(struct xsk_socket_info *xsk)
while (ret != rcvd) {
if (ret < 0)
exit_with_error(-ret);
+ if (xsk_ring_prod__needs_wakeup(&xsk->umem->fq))
+ ret = poll(fds, num_socks, opt_timeout);
ret = xsk_ring_prod__reserve(&xsk->umem->fq, rcvd,
&idx_fq);
}
@@ -505,7 +518,8 @@ static inline void complete_tx_only(struct xsk_socket_info *xsk)
if (!xsk->outstanding_tx)
return;
- kick_tx(xsk);
+ if (!opt_might_sleep || xsk_ring_prod__needs_wakeup(&xsk->tx))
+ kick_tx(xsk);
rcvd = xsk_ring_cons__peek(&xsk->umem->cq, BATCH_SIZE, &idx);
if (rcvd > 0) {
@@ -515,20 +529,25 @@ static inline void complete_tx_only(struct xsk_socket_info *xsk)
}
}
-static void rx_drop(struct xsk_socket_info *xsk)
+static void rx_drop(struct xsk_socket_info *xsk, struct pollfd *fds)
{
unsigned int rcvd, i;
u32 idx_rx = 0, idx_fq = 0;
int ret;
rcvd = xsk_ring_cons__peek(&xsk->rx, BATCH_SIZE, &idx_rx);
- if (!rcvd)
+ if (!rcvd) {
+ if (xsk_ring_prod__needs_wakeup(&xsk->umem->fq))
+ ret = poll(fds, num_socks, opt_timeout);
return;
+ }
ret = xsk_ring_prod__reserve(&xsk->umem->fq, rcvd, &idx_fq);
while (ret != rcvd) {
if (ret < 0)
exit_with_error(-ret);
+ if (xsk_ring_prod__needs_wakeup(&xsk->umem->fq))
+ ret = poll(fds, num_socks, opt_timeout);
ret = xsk_ring_prod__reserve(&xsk->umem->fq, rcvd, &idx_fq);
}
@@ -549,42 +568,65 @@ static void rx_drop(struct xsk_socket_info *xsk)
static void rx_drop_all(void)
{
struct pollfd fds[MAX_SOCKS + 1];
- int i, ret, timeout, nfds = 1;
+ int i, ret;
memset(fds, 0, sizeof(fds));
for (i = 0; i < num_socks; i++) {
fds[i].fd = xsk_socket__fd(xsks[i]->xsk);
fds[i].events = POLLIN;
- timeout = 1000; /* 1sn */
}
for (;;) {
if (opt_poll) {
- ret = poll(fds, nfds, timeout);
+ ret = poll(fds, num_socks, opt_timeout);
if (ret <= 0)
continue;
}
for (i = 0; i < num_socks; i++)
- rx_drop(xsks[i]);
+ rx_drop(xsks[i], fds);
+ }
+}
+
+static void tx_only(struct xsk_socket_info *xsk, u32 frame_nb)
+{
+ u32 idx;
+
+ if (xsk_ring_prod__reserve(&xsk->tx, BATCH_SIZE, &idx) == BATCH_SIZE) {
+ unsigned int i;
+
+ for (i = 0; i < BATCH_SIZE; i++) {
+ xsk_ring_prod__tx_desc(&xsk->tx, idx + i)->addr =
+ (frame_nb + i) << XSK_UMEM__DEFAULT_FRAME_SHIFT;
+ xsk_ring_prod__tx_desc(&xsk->tx, idx + i)->len =
+ sizeof(pkt_data) - 1;
+ }
+
+ xsk_ring_prod__submit(&xsk->tx, BATCH_SIZE);
+ xsk->outstanding_tx += BATCH_SIZE;
+ frame_nb += BATCH_SIZE;
+ frame_nb %= NUM_FRAMES;
}
+
+ complete_tx_only(xsk);
}
-static void tx_only(struct xsk_socket_info *xsk)
+static void tx_only_all(void)
{
- int timeout, ret, nfds = 1;
- struct pollfd fds[nfds + 1];
- u32 idx, frame_nb = 0;
+ struct pollfd fds[MAX_SOCKS];
+ u32 frame_nb[MAX_SOCKS] = {};
+ int i, ret;
memset(fds, 0, sizeof(fds));
- fds[0].fd = xsk_socket__fd(xsk->xsk);
- fds[0].events = POLLOUT;
- timeout = 1000; /* 1sn */
+ for (i = 0; i < num_socks; i++) {
+ fds[0].fd = xsk_socket__fd(xsks[i]->xsk);
+ fds[0].events = POLLOUT;
+ }
for (;;) {
if (opt_poll) {
- ret = poll(fds, nfds, timeout);
+ ret = poll(fds, num_socks, opt_timeout);
if (ret <= 0)
continue;
@@ -592,69 +634,75 @@ static void tx_only(struct xsk_socket_info *xsk)
continue;
}
- if (xsk_ring_prod__reserve(&xsk->tx, BATCH_SIZE, &idx) ==
- BATCH_SIZE) {
- unsigned int i;
-
- for (i = 0; i < BATCH_SIZE; i++) {
- xsk_ring_prod__tx_desc(&xsk->tx, idx + i)->addr
- = (frame_nb + i) * opt_xsk_frame_size;
- xsk_ring_prod__tx_desc(&xsk->tx, idx + i)->len =
- sizeof(pkt_data) - 1;
- }
-
- xsk_ring_prod__submit(&xsk->tx, BATCH_SIZE);
- xsk->outstanding_tx += BATCH_SIZE;
- frame_nb += BATCH_SIZE;
- frame_nb %= NUM_FRAMES;
- }
-
- complete_tx_only(xsk);
+ for (i = 0; i < num_socks; i++)
+ tx_only(xsks[i], frame_nb[i]);
}
}
-static void l2fwd(struct xsk_socket_info *xsk)
+static void l2fwd(struct xsk_socket_info *xsk, struct pollfd *fds)
{
- for (;;) {
- unsigned int rcvd, i;
- u32 idx_rx = 0, idx_tx = 0;
- int ret;
+ unsigned int rcvd, i;
+ u32 idx_rx = 0, idx_tx = 0;
+ int ret;
- for (;;) {
- complete_tx_l2fwd(xsk);
+ complete_tx_l2fwd(xsk, fds);
- rcvd = xsk_ring_cons__peek(&xsk->rx, BATCH_SIZE,
- &idx_rx);
- if (rcvd > 0)
- break;
- }
+ rcvd = xsk_ring_cons__peek(&xsk->rx, BATCH_SIZE, &idx_rx);
+ if (!rcvd) {
+ if (xsk_ring_prod__needs_wakeup(&xsk->umem->fq))
+ ret = poll(fds, num_socks, opt_timeout);
+ return;
+ }
+ ret = xsk_ring_prod__reserve(&xsk->tx, rcvd, &idx_tx);
+ while (ret != rcvd) {
+ if (ret < 0)
+ exit_with_error(-ret);
+ if (xsk_ring_prod__needs_wakeup(&xsk->tx))
+ kick_tx(xsk);
ret = xsk_ring_prod__reserve(&xsk->tx, rcvd, &idx_tx);
- while (ret != rcvd) {
- if (ret < 0)
- exit_with_error(-ret);
- ret = xsk_ring_prod__reserve(&xsk->tx, rcvd, &idx_tx);
- }
+ }
+
+ for (i = 0; i < rcvd; i++) {
+ u64 addr = xsk_ring_cons__rx_desc(&xsk->rx, idx_rx)->addr;
+ u32 len = xsk_ring_cons__rx_desc(&xsk->rx, idx_rx++)->len;
+ char *pkt = xsk_umem__get_data(xsk->umem->buffer, addr);
+
+ swap_mac_addresses(pkt);
- for (i = 0; i < rcvd; i++) {
- u64 addr = xsk_ring_cons__rx_desc(&xsk->rx,
- idx_rx)->addr;
- u32 len = xsk_ring_cons__rx_desc(&xsk->rx,
- idx_rx++)->len;
- char *pkt = xsk_umem__get_data(xsk->umem->buffer, addr);
+ hex_dump(pkt, len, addr);
+ xsk_ring_prod__tx_desc(&xsk->tx, idx_tx)->addr = addr;
+ xsk_ring_prod__tx_desc(&xsk->tx, idx_tx++)->len = len;
+ }
- swap_mac_addresses(pkt);
+ xsk_ring_prod__submit(&xsk->tx, rcvd);
+ xsk_ring_cons__release(&xsk->rx, rcvd);
- hex_dump(pkt, len, addr);
- xsk_ring_prod__tx_desc(&xsk->tx, idx_tx)->addr = addr;
- xsk_ring_prod__tx_desc(&xsk->tx, idx_tx++)->len = len;
- }
+ xsk->rx_npkts += rcvd;
+ xsk->outstanding_tx += rcvd;
+}
+
+static void l2fwd_all(void)
+{
+ struct pollfd fds[MAX_SOCKS];
+ int i, ret;
+
+ memset(fds, 0, sizeof(fds));
+
+ for (i = 0; i < num_socks; i++) {
+ fds[i].fd = xsk_socket__fd(xsks[i]->xsk);
+ fds[i].events = POLLOUT | POLLIN;
+ }
- xsk_ring_prod__submit(&xsk->tx, rcvd);
- xsk_ring_cons__release(&xsk->rx, rcvd);
+ for (;;) {
+ if (opt_poll) {
+ ret = poll(fds, num_socks, opt_timeout);
+ if (ret <= 0)
+ continue;
+ }
- xsk->rx_npkts += rcvd;
- xsk->outstanding_tx += rcvd;
+ for (i = 0; i < num_socks; i++)
+ l2fwd(xsks[i], fds);
}
}
@@ -705,9 +753,9 @@ int main(int argc, char **argv)
if (opt_bench == BENCH_RXDROP)
rx_drop_all();
else if (opt_bench == BENCH_TXONLY)
- tx_only(xsks[0]);
+ tx_only_all();
else
- l2fwd(xsks[0]);
+ l2fwd_all();
return 0;
}
--
2.7.4
^ permalink raw reply related
* [PATCH bpf-next v3 5/6] libbpf: add support for need_wakeup flag in AF_XDP part
From: Magnus Karlsson @ 2019-07-04 12:42 UTC (permalink / raw)
To: magnus.karlsson, bjorn.topel, ast, daniel, netdev, brouer
Cc: bpf, bruce.richardson, ciara.loftus, jakub.kicinski, xiaolong.ye,
qi.z.zhang, maximmi, sridhar.samudrala, kevin.laatz,
ilias.apalodimas, kiran.patil, axboe, maciej.fijalkowski,
maciejromanfijalkowski, intel-wired-lan
In-Reply-To: <1562244134-19069-1-git-send-email-magnus.karlsson@intel.com>
This commit adds support for the new need_wakeup flag in AF_XDP. The
xsk_socket__create function is updated to handle this and a new
function is introduced called xsk_ring_prod__needs_wakeup(). This
function can be used by the application to check if Rx and/or Tx
processing needs to be explicitly woken up.
Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
tools/include/uapi/linux/if_xdp.h | 13 +++++++++++++
tools/lib/bpf/xsk.c | 4 ++++
tools/lib/bpf/xsk.h | 6 ++++++
3 files changed, 23 insertions(+)
diff --git a/tools/include/uapi/linux/if_xdp.h b/tools/include/uapi/linux/if_xdp.h
index faaa5ca..62b80d5 100644
--- a/tools/include/uapi/linux/if_xdp.h
+++ b/tools/include/uapi/linux/if_xdp.h
@@ -16,6 +16,15 @@
#define XDP_SHARED_UMEM (1 << 0)
#define XDP_COPY (1 << 1) /* Force copy-mode */
#define XDP_ZEROCOPY (1 << 2) /* Force zero-copy mode */
+/* If this option is set, the driver might go sleep and in that case
+ * the XDP_RING_NEED_WAKEUP flag in the fill and/or Tx rings will be
+ * set. If it is set, the application need to explicitly wake up the
+ * driver with a poll() (Rx and Tx) or sendto() (Tx only). If you are
+ * running the driver and the application on the same core, you should
+ * use this option so that the kernel will yield to the user space
+ * application.
+ */
+#define XDP_USE_NEED_WAKEUP (1 << 3)
struct sockaddr_xdp {
__u16 sxdp_family;
@@ -25,10 +34,14 @@ struct sockaddr_xdp {
__u32 sxdp_shared_umem_fd;
};
+/* XDP_RING flags */
+#define XDP_RING_NEED_WAKEUP (1 << 0)
+
struct xdp_ring_offset {
__u64 producer;
__u64 consumer;
__u64 desc;
+ __u64 flags;
};
struct xdp_mmap_offsets {
diff --git a/tools/lib/bpf/xsk.c b/tools/lib/bpf/xsk.c
index b337402..5665b4d 100644
--- a/tools/lib/bpf/xsk.c
+++ b/tools/lib/bpf/xsk.c
@@ -224,6 +224,7 @@ int xsk_umem__create(struct xsk_umem **umem_ptr, void *umem_area, __u64 size,
fill->size = umem->config.fill_size;
fill->producer = map + off.fr.producer;
fill->consumer = map + off.fr.consumer;
+ fill->flags = map + off.fr.flags;
fill->ring = map + off.fr.desc;
fill->cached_cons = umem->config.fill_size;
@@ -241,6 +242,7 @@ int xsk_umem__create(struct xsk_umem **umem_ptr, void *umem_area, __u64 size,
comp->size = umem->config.comp_size;
comp->producer = map + off.cr.producer;
comp->consumer = map + off.cr.consumer;
+ comp->flags = map + off.cr.flags;
comp->ring = map + off.cr.desc;
*umem_ptr = umem;
@@ -564,6 +566,7 @@ int xsk_socket__create(struct xsk_socket **xsk_ptr, const char *ifname,
rx->size = xsk->config.rx_size;
rx->producer = rx_map + off.rx.producer;
rx->consumer = rx_map + off.rx.consumer;
+ rx->flags = rx_map + off.rx.flags;
rx->ring = rx_map + off.rx.desc;
}
xsk->rx = rx;
@@ -583,6 +586,7 @@ int xsk_socket__create(struct xsk_socket **xsk_ptr, const char *ifname,
tx->size = xsk->config.tx_size;
tx->producer = tx_map + off.tx.producer;
tx->consumer = tx_map + off.tx.consumer;
+ tx->flags = tx_map + off.tx.flags;
tx->ring = tx_map + off.tx.desc;
tx->cached_cons = xsk->config.tx_size;
}
diff --git a/tools/lib/bpf/xsk.h b/tools/lib/bpf/xsk.h
index 833a6e6..aa1d612 100644
--- a/tools/lib/bpf/xsk.h
+++ b/tools/lib/bpf/xsk.h
@@ -32,6 +32,7 @@ struct name { \
__u32 *producer; \
__u32 *consumer; \
void *ring; \
+ __u32 *flags; \
}
DEFINE_XSK_RING(xsk_ring_prod);
@@ -76,6 +77,11 @@ xsk_ring_cons__rx_desc(const struct xsk_ring_cons *rx, __u32 idx)
return &descs[idx & rx->mask];
}
+static inline int xsk_ring_prod__needs_wakeup(const struct xsk_ring_prod *r)
+{
+ return *r->flags & XDP_RING_NEED_WAKEUP;
+}
+
static inline __u32 xsk_prod_nb_free(struct xsk_ring_prod *r, __u32 nb)
{
__u32 free_entries = r->cached_cons - r->cached_prod;
--
2.7.4
^ permalink raw reply related
* [PATCH bpf-next v3 4/6] ixgbe: add support for AF_XDP need_wakup feature
From: Magnus Karlsson @ 2019-07-04 12:42 UTC (permalink / raw)
To: magnus.karlsson, bjorn.topel, ast, daniel, netdev, brouer
Cc: bpf, bruce.richardson, ciara.loftus, jakub.kicinski, xiaolong.ye,
qi.z.zhang, maximmi, sridhar.samudrala, kevin.laatz,
ilias.apalodimas, kiran.patil, axboe, maciej.fijalkowski,
maciejromanfijalkowski, intel-wired-lan
In-Reply-To: <1562244134-19069-1-git-send-email-magnus.karlsson@intel.com>
This patch adds support for the need_wakeup feature of AF_XDP. If the
application has told the kernel that it might sleep using the new bind
flag XDP_USE_NEED_WAKEUP, the driver will then set this flag if it has
no more buffers on the NIC Rx ring and yield to the application. For
Tx, it will set the flag if it has no outstanding Tx completion
interrupts and return to the application.
Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
index e598af9..3fbc4e4 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
@@ -547,6 +547,14 @@ int ixgbe_clean_rx_irq_zc(struct ixgbe_q_vector *q_vector,
q_vector->rx.total_packets += total_rx_packets;
q_vector->rx.total_bytes += total_rx_bytes;
+ if (xsk_umem_uses_might_sleep(rx_ring->xsk_umem)) {
+ if (failure || rx_ring->next_to_clean == rx_ring->next_to_use)
+ xsk_set_rx_need_wakeup(rx_ring->xsk_umem);
+ else
+ xsk_clear_rx_need_wakeup(rx_ring->xsk_umem);
+
+ return (int)total_rx_packets;
+ }
return failure ? budget : (int)total_rx_packets;
}
@@ -689,6 +697,14 @@ bool ixgbe_clean_xdp_tx_irq(struct ixgbe_q_vector *q_vector,
xsk_umem_complete_tx(umem, xsk_frames);
xmit_done = ixgbe_xmit_zc(tx_ring, q_vector->tx.work_limit);
+
+ if (xsk_umem_uses_might_sleep(tx_ring->xsk_umem)) {
+ if (tx_ring->next_to_clean == tx_ring->next_to_use)
+ xsk_set_tx_need_wakeup(tx_ring->xsk_umem);
+ else
+ xsk_clear_tx_need_wakeup(tx_ring->xsk_umem);
+ }
+
return budget > 0 && xmit_done;
}
--
2.7.4
^ permalink raw reply related
* [PATCH bpf-next v3 3/6] i40e: add support for AF_XDP need_wakup feature
From: Magnus Karlsson @ 2019-07-04 12:42 UTC (permalink / raw)
To: magnus.karlsson, bjorn.topel, ast, daniel, netdev, brouer
Cc: bpf, bruce.richardson, ciara.loftus, jakub.kicinski, xiaolong.ye,
qi.z.zhang, maximmi, sridhar.samudrala, kevin.laatz,
ilias.apalodimas, kiran.patil, axboe, maciej.fijalkowski,
maciejromanfijalkowski, intel-wired-lan
In-Reply-To: <1562244134-19069-1-git-send-email-magnus.karlsson@intel.com>
This patch adds support for the need_wakeup feature of AF_XDP. If the
application has told the kernel that it might sleep using the new bind
flag XDP_USE_NEED_WAKEUP, the driver will then set this flag if it has
no more buffers on the NIC Rx ring and yield to the application. For
Tx, it will set the flag if it has no outstanding Tx completion
interrupts and return to the application.
Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e_xsk.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_xsk.c b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
index d0ff5d8..18de7b6 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_xsk.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
@@ -626,6 +626,15 @@ int i40e_clean_rx_irq_zc(struct i40e_ring *rx_ring, int budget)
i40e_finalize_xdp_rx(rx_ring, xdp_xmit);
i40e_update_rx_stats(rx_ring, total_rx_bytes, total_rx_packets);
+
+ if (xsk_umem_uses_might_sleep(rx_ring->xsk_umem)) {
+ if (failure || rx_ring->next_to_clean == rx_ring->next_to_use)
+ xsk_set_rx_need_wakeup(rx_ring->xsk_umem);
+ else
+ xsk_clear_rx_need_wakeup(rx_ring->xsk_umem);
+
+ return (int)total_rx_packets;
+ }
return failure ? budget : (int)total_rx_packets;
}
@@ -761,6 +770,13 @@ bool i40e_clean_xdp_tx_irq(struct i40e_vsi *vsi,
out_xmit:
xmit_done = i40e_xmit_zc(tx_ring, budget);
+ if (xsk_umem_uses_might_sleep(tx_ring->xsk_umem)) {
+ if (tx_ring->next_to_clean == tx_ring->next_to_use)
+ xsk_set_tx_need_wakeup(tx_ring->xsk_umem);
+ else
+ xsk_clear_tx_need_wakeup(tx_ring->xsk_umem);
+ }
+
return work_done && xmit_done;
}
--
2.7.4
^ permalink raw reply related
* [PATCH bpf-next v3 2/6] xsk: add support for need_wakeup flag in AF_XDP rings
From: Magnus Karlsson @ 2019-07-04 12:42 UTC (permalink / raw)
To: magnus.karlsson, bjorn.topel, ast, daniel, netdev, brouer
Cc: bpf, bruce.richardson, ciara.loftus, jakub.kicinski, xiaolong.ye,
qi.z.zhang, maximmi, sridhar.samudrala, kevin.laatz,
ilias.apalodimas, kiran.patil, axboe, maciej.fijalkowski,
maciejromanfijalkowski, intel-wired-lan
In-Reply-To: <1562244134-19069-1-git-send-email-magnus.karlsson@intel.com>
This commit adds support for a new flag called need_wakeup in the
AF_XDP Tx and fill rings. When this flag is set, it means that the
application has to explicitly wake up the kernel Rx (for the bit in
the fill ring) or kernel Tx (for bit in the Tx ring) processing by
issuing a syscall. Poll() can wake up both depending on the flags
submitted and sendto() will wake up tx processing only.
The main reason for introducing this new flag is to be able to
efficiently support the case when application and driver is executing
on the same core. Previously, the driver was just busy-spinning on the
fill ring if it ran out of buffers in the HW and there were none on
the fill ring. This approach works when the application is running on
another core as it can replenish the fill ring while the driver is
busy-spinning. Though, this is a lousy approach if both of them are
running on the same core as the probability of the fill ring getting
more entries when the driver is busy-spinning is zero. With this new
feature the driver now sets the need_wakeup flag and returns to the
application. The application can then replenish the fill queue and
then explicitly wake up the Rx processing in the kernel using the
syscall poll(). For Tx, the flag is only set to one if the driver has
no outstanding Tx completion interrupts. If it has some, the flag is
zero as it will be woken up by a completion interrupt anyway.
As a nice side effect, this new flag also improves the performance of
the case where application and driver are running on two different
cores as it reduces the number of syscalls to the kernel. The kernel
tells user space if it needs to be woken up by a syscall, and this
eliminates many of the syscalls.
This flag needs some simple driver support. If the driver does not
support this, the Rx flag is always zero and the Tx flag is always
one. This makes any application relying on this feature default to the
old behaviour of not requiring any syscalls in the Rx path and always
having to call sendto() in the Tx path.
For backwards compatibility reasons, this feature has to be explicitly
turned on using a new bind flag (XDP_USE_NEED_WAKEUP). I recommend
that you always turn it on as it so far always have had a positive
performance impact.
The name and inspiration of the flag has been taken from io_uring by
Jens Axboe. Details about this feature in io_uring can be found in
http://kernel.dk/io_uring.pdf, section 8.3.
Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
include/linux/netdevice.h | 4 ++
include/net/xdp_sock.h | 33 +++++++++-
include/uapi/linux/if_xdp.h | 13 ++++
net/xdp/xdp_umem.c | 9 +++
net/xdp/xsk.c | 146 ++++++++++++++++++++++++++++++++++++++------
net/xdp/xsk.h | 13 ++++
net/xdp/xsk_queue.h | 1 +
7 files changed, 199 insertions(+), 20 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 60eef29..a206c1d 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -872,6 +872,9 @@ struct bpf_prog_offload_ops;
struct netlink_ext_ack;
struct xdp_umem;
+/* Flags for the flags field in XDP_SETUP_XSK_UMEM */
+#define XSK_DRV_CAN_SLEEP (1 << 0) /* The driver is alowed to sleep. */
+
struct netdev_bpf {
enum bpf_netdev_command command;
union {
@@ -894,6 +897,7 @@ struct netdev_bpf {
/* XDP_SETUP_XSK_UMEM */
struct {
struct xdp_umem *umem;
+ u32 flags;
u16 queue_id;
} xsk;
};
diff --git a/include/net/xdp_sock.h b/include/net/xdp_sock.h
index 057b159..66c3755 100644
--- a/include/net/xdp_sock.h
+++ b/include/net/xdp_sock.h
@@ -27,6 +27,9 @@ struct xdp_umem_fq_reuse {
u64 handles[];
};
+/* Flags for the umem flags field. */
+#define XDP_UMEM_MIGHT_SLEEP (1 << 0)
+
struct xdp_umem {
struct xsk_queue *fq;
struct xsk_queue *cq;
@@ -41,10 +44,12 @@ struct xdp_umem {
struct work_struct work;
struct page **pgs;
u32 npgs;
+ u16 queue_id;
+ u8 need_wakeup;
+ u8 flags;
int id;
struct net_device *dev;
struct xdp_umem_fq_reuse *fq_reuse;
- u16 queue_id;
bool zc;
spinlock_t xsk_list_lock;
struct list_head xsk_list;
@@ -88,6 +93,11 @@ struct xdp_umem_fq_reuse *xsk_reuseq_swap(struct xdp_umem *umem,
struct xdp_umem_fq_reuse *newq);
void xsk_reuseq_free(struct xdp_umem_fq_reuse *rq);
struct xdp_umem *xdp_get_umem_from_qid(struct net_device *dev, u16 queue_id);
+void xsk_set_rx_need_wakeup(struct xdp_umem *umem);
+void xsk_set_tx_need_wakeup(struct xdp_umem *umem);
+void xsk_clear_rx_need_wakeup(struct xdp_umem *umem);
+void xsk_clear_tx_need_wakeup(struct xdp_umem *umem);
+bool xsk_umem_uses_might_sleep(struct xdp_umem *umem);
static inline char *xdp_umem_get_data(struct xdp_umem *umem, u64 addr)
{
@@ -234,6 +244,27 @@ static inline void xsk_umem_fq_reuse(struct xdp_umem *umem, u64 addr)
{
}
+static inline void xsk_set_rx_need_wakeup(struct xdp_umem *umem)
+{
+}
+
+static inline void xsk_set_tx_need_wakeup(struct xdp_umem *umem)
+{
+}
+
+static inline void xsk_clear_rx_need_wakeup(struct xdp_umem *umem)
+{
+}
+
+static inline void xsk_clear_tx_need_wakeup(struct xdp_umem *umem)
+{
+}
+
+static inline bool xsk_umem_uses_might_sleep(struct xdp_umem *umem)
+{
+ return false;
+}
+
#endif /* CONFIG_XDP_SOCKETS */
#endif /* _LINUX_XDP_SOCK_H */
diff --git a/include/uapi/linux/if_xdp.h b/include/uapi/linux/if_xdp.h
index faaa5ca..62b80d5 100644
--- a/include/uapi/linux/if_xdp.h
+++ b/include/uapi/linux/if_xdp.h
@@ -16,6 +16,15 @@
#define XDP_SHARED_UMEM (1 << 0)
#define XDP_COPY (1 << 1) /* Force copy-mode */
#define XDP_ZEROCOPY (1 << 2) /* Force zero-copy mode */
+/* If this option is set, the driver might go sleep and in that case
+ * the XDP_RING_NEED_WAKEUP flag in the fill and/or Tx rings will be
+ * set. If it is set, the application need to explicitly wake up the
+ * driver with a poll() (Rx and Tx) or sendto() (Tx only). If you are
+ * running the driver and the application on the same core, you should
+ * use this option so that the kernel will yield to the user space
+ * application.
+ */
+#define XDP_USE_NEED_WAKEUP (1 << 3)
struct sockaddr_xdp {
__u16 sxdp_family;
@@ -25,10 +34,14 @@ struct sockaddr_xdp {
__u32 sxdp_shared_umem_fd;
};
+/* XDP_RING flags */
+#define XDP_RING_NEED_WAKEUP (1 << 0)
+
struct xdp_ring_offset {
__u64 producer;
__u64 consumer;
__u64 desc;
+ __u64 flags;
};
struct xdp_mmap_offsets {
diff --git a/net/xdp/xdp_umem.c b/net/xdp/xdp_umem.c
index 803554b..6b05cd0 100644
--- a/net/xdp/xdp_umem.c
+++ b/net/xdp/xdp_umem.c
@@ -105,6 +105,15 @@ int xdp_umem_assign_dev(struct xdp_umem *umem, struct net_device *dev,
umem->dev = dev;
umem->queue_id = queue_id;
+ if (flags & XDP_USE_NEED_WAKEUP) {
+ umem->flags |= XDP_UMEM_MIGHT_SLEEP;
+ /* Tx needs to be explicitly woken up the first time.
+ * Also for supporting drivers that do not implement this
+ * feature. They will always have to call sendto().
+ */
+ xsk_set_tx_need_wakeup(umem);
+ }
+
if (force_copy)
/* For copy-mode, we are done. */
goto out_rtnl_unlock;
diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
index cf8898f..ea2143f 100644
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -55,6 +55,66 @@ void xsk_umem_discard_addr(struct xdp_umem *umem)
}
EXPORT_SYMBOL(xsk_umem_discard_addr);
+void xsk_set_rx_need_wakeup(struct xdp_umem *umem)
+{
+ if (umem->need_wakeup & XDP_WAKEUP_RX)
+ return;
+
+ umem->fq->ring->flags |= XDP_RING_NEED_WAKEUP;
+ umem->need_wakeup |= XDP_WAKEUP_RX;
+}
+EXPORT_SYMBOL(xsk_set_rx_need_wakeup);
+
+void xsk_set_tx_need_wakeup(struct xdp_umem *umem)
+{
+ struct xdp_sock *xs;
+
+ if (umem->need_wakeup & XDP_WAKEUP_TX)
+ return;
+
+ rcu_read_lock();
+ list_for_each_entry_rcu(xs, &umem->xsk_list, list) {
+ xs->tx->ring->flags |= XDP_RING_NEED_WAKEUP;
+ }
+ rcu_read_unlock();
+
+ umem->need_wakeup |= XDP_WAKEUP_TX;
+}
+EXPORT_SYMBOL(xsk_set_tx_need_wakeup);
+
+void xsk_clear_rx_need_wakeup(struct xdp_umem *umem)
+{
+ if (!(umem->need_wakeup & XDP_WAKEUP_RX))
+ return;
+
+ umem->fq->ring->flags &= ~XDP_RING_NEED_WAKEUP;
+ umem->need_wakeup &= ~XDP_WAKEUP_RX;
+}
+EXPORT_SYMBOL(xsk_clear_rx_need_wakeup);
+
+void xsk_clear_tx_need_wakeup(struct xdp_umem *umem)
+{
+ struct xdp_sock *xs;
+
+ if (!(umem->need_wakeup & XDP_WAKEUP_TX))
+ return;
+
+ rcu_read_lock();
+ list_for_each_entry_rcu(xs, &umem->xsk_list, list) {
+ xs->tx->ring->flags &= ~XDP_RING_NEED_WAKEUP;
+ }
+ rcu_read_unlock();
+
+ umem->need_wakeup &= ~XDP_WAKEUP_TX;
+}
+EXPORT_SYMBOL(xsk_clear_tx_need_wakeup);
+
+bool xsk_umem_uses_might_sleep(struct xdp_umem *umem)
+{
+ return umem->flags & XDP_UMEM_MIGHT_SLEEP;
+}
+EXPORT_SYMBOL(xsk_umem_uses_might_sleep);
+
static int __xsk_rcv(struct xdp_sock *xs, struct xdp_buff *xdp, u32 len)
{
void *to_buf, *from_buf;
@@ -311,6 +371,12 @@ static unsigned int xsk_poll(struct file *file, struct socket *sock,
unsigned int mask = datagram_poll(file, sock, wait);
struct sock *sk = sock->sk;
struct xdp_sock *xs = xdp_sk(sk);
+ struct net_device *dev = xs->dev;
+ struct xdp_umem *umem = xs->umem;
+
+ if (umem->need_wakeup)
+ dev->netdev_ops->ndo_xsk_wakeup(dev, xs->queue_id,
+ umem->need_wakeup);
if (xs->rx && !xskq_empty_desc(xs->rx))
mask |= POLLIN | POLLRDNORM;
@@ -411,7 +477,8 @@ static int xsk_bind(struct socket *sock, struct sockaddr *addr, int addr_len)
return -EINVAL;
flags = sxdp->sxdp_flags;
- if (flags & ~(XDP_SHARED_UMEM | XDP_COPY | XDP_ZEROCOPY))
+ if (flags & ~(XDP_SHARED_UMEM | XDP_COPY | XDP_ZEROCOPY |
+ XDP_USE_NEED_WAKEUP))
return -EINVAL;
mutex_lock(&xs->mutex);
@@ -437,7 +504,8 @@ static int xsk_bind(struct socket *sock, struct sockaddr *addr, int addr_len)
struct xdp_sock *umem_xs;
struct socket *sock;
- if ((flags & XDP_COPY) || (flags & XDP_ZEROCOPY)) {
+ if ((flags & XDP_COPY) || (flags & XDP_ZEROCOPY) ||
+ (flags & XDP_USE_NEED_WAKEUP)) {
/* Cannot specify flags for shared sockets. */
err = -EINVAL;
goto out_unlock;
@@ -525,6 +593,9 @@ static int xsk_setsockopt(struct socket *sock, int level, int optname,
mutex_lock(&xs->mutex);
q = (optname == XDP_TX_RING) ? &xs->tx : &xs->rx;
err = xsk_init_queue(entries, q, false);
+ if (!err && optname == XDP_TX_RING)
+ /* Tx needs to be explicitly woken up the first time */
+ xs->tx->ring->flags |= XDP_RING_NEED_WAKEUP;
mutex_unlock(&xs->mutex);
return err;
}
@@ -582,6 +653,20 @@ static int xsk_setsockopt(struct socket *sock, int level, int optname,
return -ENOPROTOOPT;
}
+static void xsk_enter_rxtx_offsets(struct xdp_ring_offset_v1 *ring)
+{
+ ring->producer = offsetof(struct xdp_rxtx_ring, ptrs.producer);
+ ring->consumer = offsetof(struct xdp_rxtx_ring, ptrs.consumer);
+ ring->desc = offsetof(struct xdp_rxtx_ring, desc);
+}
+
+static void xsk_enter_umem_offsets(struct xdp_ring_offset_v1 *ring)
+{
+ ring->producer = offsetof(struct xdp_umem_ring, ptrs.producer);
+ ring->consumer = offsetof(struct xdp_umem_ring, ptrs.consumer);
+ ring->desc = offsetof(struct xdp_umem_ring, desc);
+}
+
static int xsk_getsockopt(struct socket *sock, int level, int optname,
char __user *optval, int __user *optlen)
{
@@ -621,26 +706,49 @@ static int xsk_getsockopt(struct socket *sock, int level, int optname,
case XDP_MMAP_OFFSETS:
{
struct xdp_mmap_offsets off;
+ struct xdp_mmap_offsets_v1 off_v1;
+ bool flags_supported = true;
+ void *to_copy;
- if (len < sizeof(off))
+ if (len < sizeof(off_v1))
return -EINVAL;
+ else if (len < sizeof(off))
+ flags_supported = false;
+
+ if (flags_supported) {
+ /* xdp_ring_offset is identical to xdp_ring_offset_v1
+ * except for the flags field added to the end.
+ */
+ xsk_enter_rxtx_offsets((struct xdp_ring_offset_v1 *)
+ &off.rx);
+ xsk_enter_rxtx_offsets((struct xdp_ring_offset_v1 *)
+ &off.tx);
+ xsk_enter_umem_offsets((struct xdp_ring_offset_v1 *)
+ &off.fr);
+ xsk_enter_umem_offsets((struct xdp_ring_offset_v1 *)
+ &off.cr);
+ off.rx.flags = offsetof(struct xdp_rxtx_ring,
+ ptrs.flags);
+ off.tx.flags = offsetof(struct xdp_rxtx_ring,
+ ptrs.flags);
+ off.fr.flags = offsetof(struct xdp_umem_ring,
+ ptrs.flags);
+ off.cr.flags = offsetof(struct xdp_umem_ring,
+ ptrs.flags);
+
+ len = sizeof(off);
+ to_copy = &off;
+ } else {
+ xsk_enter_rxtx_offsets(&off_v1.rx);
+ xsk_enter_rxtx_offsets(&off_v1.tx);
+ xsk_enter_umem_offsets(&off_v1.fr);
+ xsk_enter_umem_offsets(&off_v1.cr);
+
+ len = sizeof(off_v1);
+ to_copy = &off_v1;
+ }
- off.rx.producer = offsetof(struct xdp_rxtx_ring, ptrs.producer);
- off.rx.consumer = offsetof(struct xdp_rxtx_ring, ptrs.consumer);
- off.rx.desc = offsetof(struct xdp_rxtx_ring, desc);
- off.tx.producer = offsetof(struct xdp_rxtx_ring, ptrs.producer);
- off.tx.consumer = offsetof(struct xdp_rxtx_ring, ptrs.consumer);
- off.tx.desc = offsetof(struct xdp_rxtx_ring, desc);
-
- off.fr.producer = offsetof(struct xdp_umem_ring, ptrs.producer);
- off.fr.consumer = offsetof(struct xdp_umem_ring, ptrs.consumer);
- off.fr.desc = offsetof(struct xdp_umem_ring, desc);
- off.cr.producer = offsetof(struct xdp_umem_ring, ptrs.producer);
- off.cr.consumer = offsetof(struct xdp_umem_ring, ptrs.consumer);
- off.cr.desc = offsetof(struct xdp_umem_ring, desc);
-
- len = sizeof(off);
- if (copy_to_user(optval, &off, len))
+ if (copy_to_user(optval, to_copy, len))
return -EFAULT;
if (put_user(len, optlen))
return -EFAULT;
diff --git a/net/xdp/xsk.h b/net/xdp/xsk.h
index ba81206..4cfd106 100644
--- a/net/xdp/xsk.h
+++ b/net/xdp/xsk.h
@@ -4,6 +4,19 @@
#ifndef XSK_H_
#define XSK_H_
+struct xdp_ring_offset_v1 {
+ __u64 producer;
+ __u64 consumer;
+ __u64 desc;
+};
+
+struct xdp_mmap_offsets_v1 {
+ struct xdp_ring_offset_v1 rx;
+ struct xdp_ring_offset_v1 tx;
+ struct xdp_ring_offset_v1 fr;
+ struct xdp_ring_offset_v1 cr;
+};
+
static inline struct xdp_sock *xdp_sk(struct sock *sk)
{
return (struct xdp_sock *)sk;
diff --git a/net/xdp/xsk_queue.h b/net/xdp/xsk_queue.h
index 12b4978..25e41d4 100644
--- a/net/xdp/xsk_queue.h
+++ b/net/xdp/xsk_queue.h
@@ -16,6 +16,7 @@
struct xdp_ring {
u32 producer ____cacheline_aligned_in_smp;
u32 consumer ____cacheline_aligned_in_smp;
+ u32 flags;
};
/* Used for the RX and TX queues for packets */
--
2.7.4
^ permalink raw reply related
* [PATCH bpf-next v3 1/6] xsk: replace ndo_xsk_async_xmit with ndo_xsk_wakeup
From: Magnus Karlsson @ 2019-07-04 12:42 UTC (permalink / raw)
To: magnus.karlsson, bjorn.topel, ast, daniel, netdev, brouer
Cc: bpf, bruce.richardson, ciara.loftus, jakub.kicinski, xiaolong.ye,
qi.z.zhang, maximmi, sridhar.samudrala, kevin.laatz,
ilias.apalodimas, kiran.patil, axboe, maciej.fijalkowski,
maciejromanfijalkowski, intel-wired-lan
In-Reply-To: <1562244134-19069-1-git-send-email-magnus.karlsson@intel.com>
This commit replaces ndo_xsk_async_xmit with ndo_xsk_wakeup. This new
ndo provides the same functionality as before but with the addition of
a new flags field that is used to specifiy if Rx, Tx or both should be
woken up. The previous ndo only woke up Tx, as implied by the
name. The i40e and ixgbe drivers (which are all the supported ones)
are updated with this new interface.
This new ndo will be used by the new need_wakeup functionality of XDP
sockets that need to be able to wake up both Rx and Tx driver
processing.
Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e_main.c | 5 +++--
drivers/net/ethernet/intel/i40e/i40e_xsk.c | 7 ++++---
drivers/net/ethernet/intel/i40e/i40e_xsk.h | 2 +-
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 5 +++--
drivers/net/ethernet/intel/ixgbe/ixgbe_txrx_common.h | 2 +-
drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c | 4 ++--
drivers/net/ethernet/mellanox/mlx5/core/en/xsk/tx.c | 2 +-
drivers/net/ethernet/mellanox/mlx5/core/en/xsk/tx.h | 2 +-
drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 2 +-
include/linux/netdevice.h | 14 ++++++++++++--
net/xdp/xdp_umem.c | 3 +--
net/xdp/xsk.c | 3 ++-
12 files changed, 32 insertions(+), 19 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 7c43ec5..eee429d 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -12022,7 +12022,8 @@ static int i40e_xdp_setup(struct i40e_vsi *vsi,
if (need_reset && prog)
for (i = 0; i < vsi->num_queue_pairs; i++)
if (vsi->xdp_rings[i]->xsk_umem)
- (void)i40e_xsk_async_xmit(vsi->netdev, i);
+ (void)i40e_xsk_wakeup(vsi->netdev, i,
+ XDP_WAKEUP_RX);
return 0;
}
@@ -12344,7 +12345,7 @@ static const struct net_device_ops i40e_netdev_ops = {
.ndo_bridge_setlink = i40e_ndo_bridge_setlink,
.ndo_bpf = i40e_xdp,
.ndo_xdp_xmit = i40e_xdp_xmit,
- .ndo_xsk_async_xmit = i40e_xsk_async_xmit,
+ .ndo_xsk_wakeup = i40e_xsk_wakeup,
};
/**
diff --git a/drivers/net/ethernet/intel/i40e/i40e_xsk.c b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
index 32bad01..d0ff5d8 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_xsk.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
@@ -116,7 +116,7 @@ static int i40e_xsk_umem_enable(struct i40e_vsi *vsi, struct xdp_umem *umem,
return err;
/* Kick start the NAPI context so that receiving will start */
- err = i40e_xsk_async_xmit(vsi->netdev, qid);
+ err = i40e_xsk_wakeup(vsi->netdev, qid, XDP_WAKEUP_RX);
if (err)
return err;
}
@@ -765,13 +765,14 @@ bool i40e_clean_xdp_tx_irq(struct i40e_vsi *vsi,
}
/**
- * i40e_xsk_async_xmit - Implements the ndo_xsk_async_xmit
+ * i40e_xsk_wakeup - Implements the ndo_xsk_wakeup
* @dev: the netdevice
* @queue_id: queue id to wake up
+ * @flags: ignored in our case since we have Rx and Tx in the same NAPI.
*
* Returns <0 for errors, 0 otherwise.
**/
-int i40e_xsk_async_xmit(struct net_device *dev, u32 queue_id)
+int i40e_xsk_wakeup(struct net_device *dev, u32 queue_id, u32 flags)
{
struct i40e_netdev_priv *np = netdev_priv(dev);
struct i40e_vsi *vsi = np->vsi;
diff --git a/drivers/net/ethernet/intel/i40e/i40e_xsk.h b/drivers/net/ethernet/intel/i40e/i40e_xsk.h
index 8cc0a2e..9ed59c1 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_xsk.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_xsk.h
@@ -18,6 +18,6 @@ int i40e_clean_rx_irq_zc(struct i40e_ring *rx_ring, int budget);
bool i40e_clean_xdp_tx_irq(struct i40e_vsi *vsi,
struct i40e_ring *tx_ring, int napi_budget);
-int i40e_xsk_async_xmit(struct net_device *dev, u32 queue_id);
+int i40e_xsk_wakeup(struct net_device *dev, u32 queue_id, u32 flags);
#endif /* _I40E_XSK_H_ */
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index b613e72..574d3f9 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -10276,7 +10276,8 @@ static int ixgbe_xdp_setup(struct net_device *dev, struct bpf_prog *prog)
if (need_reset && prog)
for (i = 0; i < adapter->num_rx_queues; i++)
if (adapter->xdp_ring[i]->xsk_umem)
- (void)ixgbe_xsk_async_xmit(adapter->netdev, i);
+ (void)ixgbe_xsk_wakeup(adapter->netdev, i,
+ XDP_WAKEUP_RX);
return 0;
}
@@ -10395,7 +10396,7 @@ static const struct net_device_ops ixgbe_netdev_ops = {
.ndo_features_check = ixgbe_features_check,
.ndo_bpf = ixgbe_xdp,
.ndo_xdp_xmit = ixgbe_xdp_xmit,
- .ndo_xsk_async_xmit = ixgbe_xsk_async_xmit,
+ .ndo_xsk_wakeup = ixgbe_xsk_wakeup,
};
static void ixgbe_disable_txr_hw(struct ixgbe_adapter *adapter,
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_txrx_common.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_txrx_common.h
index d93a690..6d01700 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_txrx_common.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_txrx_common.h
@@ -42,7 +42,7 @@ int ixgbe_clean_rx_irq_zc(struct ixgbe_q_vector *q_vector,
void ixgbe_xsk_clean_rx_ring(struct ixgbe_ring *rx_ring);
bool ixgbe_clean_xdp_tx_irq(struct ixgbe_q_vector *q_vector,
struct ixgbe_ring *tx_ring, int napi_budget);
-int ixgbe_xsk_async_xmit(struct net_device *dev, u32 queue_id);
+int ixgbe_xsk_wakeup(struct net_device *dev, u32 queue_id, u32 flags);
void ixgbe_xsk_clean_tx_ring(struct ixgbe_ring *tx_ring);
#endif /* #define _IXGBE_TXRX_COMMON_H_ */
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
index 6b60955..e598af9 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
@@ -100,7 +100,7 @@ static int ixgbe_xsk_umem_enable(struct ixgbe_adapter *adapter,
ixgbe_txrx_ring_enable(adapter, qid);
/* Kick start the NAPI context so that receiving will start */
- err = ixgbe_xsk_async_xmit(adapter->netdev, qid);
+ err = ixgbe_xsk_wakeup(adapter->netdev, qid, XDP_WAKEUP_RX);
if (err)
return err;
}
@@ -692,7 +692,7 @@ bool ixgbe_clean_xdp_tx_irq(struct ixgbe_q_vector *q_vector,
return budget > 0 && xmit_done;
}
-int ixgbe_xsk_async_xmit(struct net_device *dev, u32 qid)
+int ixgbe_xsk_wakeup(struct net_device *dev, u32 qid, u32 flags)
{
struct ixgbe_adapter *adapter = netdev_priv(dev);
struct ixgbe_ring *ring;
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/tx.c b/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/tx.c
index 35e188cf..9704634 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/tx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/tx.c
@@ -7,7 +7,7 @@
#include "en/params.h"
#include <net/xdp_sock.h>
-int mlx5e_xsk_async_xmit(struct net_device *dev, u32 qid)
+int mlx5e_xsk_wakeup(struct net_device *dev, u32 qid, u32 flags)
{
struct mlx5e_priv *priv = netdev_priv(dev);
struct mlx5e_params *params = &priv->channels.params;
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/tx.h b/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/tx.h
index 7add18b..9c50515 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/tx.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/tx.h
@@ -8,7 +8,7 @@
/* TX data path */
-int mlx5e_xsk_async_xmit(struct net_device *dev, u32 qid);
+int mlx5e_xsk_wakeup(struct net_device *dev, u32 qid, u32 flags);
bool mlx5e_xsk_tx(struct mlx5e_xdpsq *sq, unsigned int budget);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index 67b562c..c862866 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -4542,7 +4542,7 @@ const struct net_device_ops mlx5e_netdev_ops = {
.ndo_tx_timeout = mlx5e_tx_timeout,
.ndo_bpf = mlx5e_xdp,
.ndo_xdp_xmit = mlx5e_xdp_xmit,
- .ndo_xsk_async_xmit = mlx5e_xsk_async_xmit,
+ .ndo_xsk_wakeup = mlx5e_xsk_wakeup,
#ifdef CONFIG_MLX5_EN_ARFS
.ndo_rx_flow_steer = mlx5e_rx_flow_steer,
#endif
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index eeacebd..60eef29 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -899,6 +899,10 @@ struct netdev_bpf {
};
};
+/* Flags for ndo_xsk_wakeup. */
+#define XDP_WAKEUP_RX (1 << 0)
+#define XDP_WAKEUP_TX (1 << 1)
+
#ifdef CONFIG_XFRM_OFFLOAD
struct xfrmdev_ops {
int (*xdo_dev_state_add) (struct xfrm_state *x);
@@ -1225,6 +1229,12 @@ struct tlsdev_ops;
* that got dropped are freed/returned via xdp_return_frame().
* Returns negative number, means general error invoking ndo, meaning
* no frames were xmit'ed and core-caller will free all frames.
+ * int (*ndo_xsk_wakeup)(struct net_device *dev, u32 queue_id, u32 flags);
+ * This function is used to wake up the softirq, ksoftirqd or kthread
+ * responsible for sending and/or receiving packets on a specific
+ * queue id bound to an AF_XDP socket. The flags field specifies if
+ * only RX, only Tx, or both should be woken up using the flags
+ * XDP_WAKEUP_RX and XDP_WAKEUP_TX.
* struct devlink_port *(*ndo_get_devlink_port)(struct net_device *dev);
* Get devlink port instance associated with a given netdev.
* Called with a reference on the netdevice and devlink locks only,
@@ -1424,8 +1434,8 @@ struct net_device_ops {
int (*ndo_xdp_xmit)(struct net_device *dev, int n,
struct xdp_frame **xdp,
u32 flags);
- int (*ndo_xsk_async_xmit)(struct net_device *dev,
- u32 queue_id);
+ int (*ndo_xsk_wakeup)(struct net_device *dev,
+ u32 queue_id, u32 flags);
struct devlink_port * (*ndo_get_devlink_port)(struct net_device *dev);
};
diff --git a/net/xdp/xdp_umem.c b/net/xdp/xdp_umem.c
index 9c6de4f..803554b 100644
--- a/net/xdp/xdp_umem.c
+++ b/net/xdp/xdp_umem.c
@@ -109,8 +109,7 @@ int xdp_umem_assign_dev(struct xdp_umem *umem, struct net_device *dev,
/* For copy-mode, we are done. */
goto out_rtnl_unlock;
- if (!dev->netdev_ops->ndo_bpf ||
- !dev->netdev_ops->ndo_xsk_async_xmit) {
+ if (!dev->netdev_ops->ndo_bpf || !dev->netdev_ops->ndo_xsk_wakeup) {
err = -EOPNOTSUPP;
goto err_unreg_umem;
}
diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
index 74417a8..cf8898f 100644
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -200,7 +200,8 @@ static int xsk_zc_xmit(struct sock *sk)
struct xdp_sock *xs = xdp_sk(sk);
struct net_device *dev = xs->dev;
- return dev->netdev_ops->ndo_xsk_async_xmit(dev, xs->queue_id);
+ return dev->netdev_ops->ndo_xsk_wakeup(dev, xs->queue_id,
+ XDP_WAKEUP_TX);
}
static void xsk_destruct_skb(struct sk_buff *skb)
--
2.7.4
^ permalink raw reply related
* [PATCH bpf-next v3 0/6] add need_wakeup flag to the AF_XDP rings
From: Magnus Karlsson @ 2019-07-04 12:42 UTC (permalink / raw)
To: magnus.karlsson, bjorn.topel, ast, daniel, netdev, brouer
Cc: bpf, bruce.richardson, ciara.loftus, jakub.kicinski, xiaolong.ye,
qi.z.zhang, maximmi, sridhar.samudrala, kevin.laatz,
ilias.apalodimas, kiran.patil, axboe, maciej.fijalkowski,
maciejromanfijalkowski, intel-wired-lan
This patch set adds support for a new flag called need_wakeup in the
AF_XDP Tx and fill rings. When this flag is set by the driver, it
means that the application has to explicitly wake up the kernel Rx
(for the bit in the fill ring) or kernel Tx (for bit in the Tx ring)
processing by issuing a syscall. Poll() can wake up both and sendto()
will wake up Tx processing only.
The main reason for introducing this new flag is to be able to
efficiently support the case when application and driver is executing
on the same core. Previously, the driver was just busy-spinning on the
fill ring if it ran out of buffers in the HW and there were none to
get from the fill ring. This approach works when the application and
driver is running on different cores as the application can replenish
the fill ring while the driver is busy-spinning. Though, this is a
lousy approach if both of them are running on the same core as the
probability of the fill ring getting more entries when the driver is
busy-spinning is zero. With this new feature the driver now sets the
need_wakeup flag and returns to the application. The application can
then replenish the fill queue and then explicitly wake up the Rx
processing in the kernel using the syscall poll(). For Tx, the flag is
only set to one if the driver has no outstanding Tx completion
interrupts. If it has some, the flag is zero as it will be woken up by
a completion interrupt anyway. This flag can also be used in other
situations where the driver needs to be woken up explicitly.
As a nice side effect, this new flag also improves the Tx performance
of the case where application and driver are running on two different
cores as it reduces the number of syscalls to the kernel. The kernel
tells user space if it needs to be woken up by a syscall, and this
eliminates many of the syscalls. The Rx performance of the 2-core case
is on the other hand slightly worse, since there is a need to use a
syscall now to wake up the driver, instead of the driver
busy-spinning. It does waste less CPU cycles though, which might lead
to better overall system performance.
This new flag needs some simple driver support. If the driver does not
support it, the Rx flag is always zero and the Tx flag is always
one. This makes any application relying on this feature default to the
old behavior of not requiring any syscalls in the Rx path and always
having to call sendto() in the Tx path.
For backwards compatibility reasons, this feature has to be explicitly
turned on using a new bind flag (XDP_USE_NEED_WAKEUP). I recommend
that you always turn it on as it has a large positive performance
impact for the one core case and does not degrade 2 core performance
and actually improves it for Tx heavy workloads.
Here are some performance numbers measured on my local,
non-performance optimized development system. That is why you are
seeing numbers lower than the ones from Björn and Jesper. 64 byte
packets at 40Gbit/s line rate. All results in Mpps. Cores == 1 means
that both application and driver is executing on the same core. Cores
== 2 that they are on different cores.
Applications
need_wakeup cores txpush rxdrop l2fwd
---------------------------------------------------------------
n 1 0.07 0.06 0.03
y 1 21.6 8.2 6.5
n 2 32.3 11.7 8.7
y 2 33.1 11.7 8.7
Overall, the need_wakeup flag provides the same or better performance
in all the micro-benchmarks. The reduction of sendto() calls in txpush
is large. Only a few per second is needed. For l2fwd, the drop is 50%
for the 1 core case and more than 99.9% for the 2 core case. Do not
know why I am not seeing the same drop for the 1 core case yet.
The name and inspiration of the flag has been taken from io_uring by
Jens Axboe. Details about this feature in io_uring can be found in
http://kernel.dk/io_uring.pdf, section 8.3. It also addresses most of
the denial of service and sendto() concerns raised by Maxim
Mikityanskiy in https://www.spinics.net/lists/netdev/msg554657.html.
The typical Tx part of an application will have to change from:
ret = sendto(fd,....)
to:
if (xsk_ring_prod__needs_wakeup(&xsk->tx))
ret = sendto(fd,....)
and th Rx part from:
rcvd = xsk_ring_cons__peek(&xsk->rx, BATCH_SIZE, &idx_rx);
if (!rcvd)
return;
to:
rcvd = xsk_ring_cons__peek(&xsk->rx, BATCH_SIZE, &idx_rx);
if (!rcvd) {
if (xsk_ring_prod__needs_wakeup(&xsk->umem->fq))
ret = poll(fd,.....);
return;
}
v2 -> v3:
* Converted the Mellanox driver to the new ndo in patch 1 as pointed out
by Maxim
* Fixed the compatibility code of XDP_MMAP_OFFSETS so it now works.
v1 -> v2:
* Fixed bisectability problem pointed out by Jakub
* Added missing initiliztion of the Tx need_wakeup flag to 1
This patch has been applied against commit e5a3e259ef23 ("Merge branch 'bpf-tcp-rtt-hook'")
Structure of the patch set:
Patch 1: Replaces the ndo_xsk_async_xmit with ndo_xsk_wakeup to
support waking up both Rx and Tx processing
Patch 2: Implements the need_wakeup functionality in common code
Patch 3-4: Add need_wakeup support to the i40e and ixgbe drivers
Patch 5: Add need_wakeup support to libbpf
Patch 6: Add need_wakeup support to the xdpsock sample application
Thanks: Magnus
Magnus Karlsson (6):
xsk: replace ndo_xsk_async_xmit with ndo_xsk_wakeup
xsk: add support for need_wakeup flag in AF_XDP rings
i40e: add support for AF_XDP need_wakup feature
ixgbe: add support for AF_XDP need_wakup feature
libbpf: add support for need_wakeup flag in AF_XDP part
samples/bpf: add use of need_sleep flag in xdpsock
drivers/net/ethernet/intel/i40e/i40e_main.c | 5 +-
drivers/net/ethernet/intel/i40e/i40e_xsk.c | 23 ++-
drivers/net/ethernet/intel/i40e/i40e_xsk.h | 2 +-
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 5 +-
.../net/ethernet/intel/ixgbe/ixgbe_txrx_common.h | 2 +-
drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c | 20 ++-
.../net/ethernet/mellanox/mlx5/core/en/xsk/tx.c | 2 +-
.../net/ethernet/mellanox/mlx5/core/en/xsk/tx.h | 2 +-
drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 2 +-
include/linux/netdevice.h | 18 +-
include/net/xdp_sock.h | 33 +++-
include/uapi/linux/if_xdp.h | 13 ++
net/xdp/xdp_umem.c | 12 +-
net/xdp/xsk.c | 149 +++++++++++++---
net/xdp/xsk.h | 13 ++
net/xdp/xsk_queue.h | 1 +
samples/bpf/xdpsock_user.c | 192 +++++++++++++--------
tools/include/uapi/linux/if_xdp.h | 13 ++
tools/lib/bpf/xsk.c | 4 +
tools/lib/bpf/xsk.h | 6 +
20 files changed, 406 insertions(+), 111 deletions(-)
--
2.7.4
^ permalink raw reply
* Re: [net-next 1/3] ice: Initialize and register platform device to provide RDMA
From: Greg KH @ 2019-07-04 12:42 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Jeff Kirsher, davem@davemloft.net, dledford@redhat.com,
Tony Nguyen, netdev@vger.kernel.org, linux-rdma@vger.kernel.org,
nhorman@redhat.com, sassmann@redhat.com, poswald@suse.com,
mustafa.ismail@intel.com, shiraz.saleem@intel.com, Dave Ertman,
Andrew Bowers
In-Reply-To: <20190704123729.GF3401@mellanox.com>
On Thu, Jul 04, 2019 at 12:37:33PM +0000, Jason Gunthorpe wrote:
> On Thu, Jul 04, 2019 at 02:29:50PM +0200, Greg KH wrote:
> > On Thu, Jul 04, 2019 at 12:16:41PM +0000, Jason Gunthorpe wrote:
> > > On Wed, Jul 03, 2019 at 07:12:50PM -0700, Jeff Kirsher wrote:
> > > > From: Tony Nguyen <anthony.l.nguyen@intel.com>
> > > >
> > > > The RDMA block does not advertise on the PCI bus or any other bus.
> > > > Thus the ice driver needs to provide access to the RDMA hardware block
> > > > via a virtual bus; utilize the platform bus to provide this access.
> > > >
> > > > This patch initializes the driver to support RDMA as well as creates
> > > > and registers a platform device for the RDMA driver to register to. At
> > > > this point the driver is fully initialized to register a platform
> > > > driver, however, can not yet register as the ops have not been
> > > > implemented.
> > >
> > > I think you need Greg's ack on all this driver stuff - particularly
> > > that a platform_device is OK.
> >
> > A platform_device is almost NEVER ok.
> >
> > Don't abuse it, make a real device on a real bus. If you don't have a
> > real bus and just need to create a device to hang other things off of,
> > then use the virtual one, that's what it is there for.
>
> Ideally I'd like to see all the RDMA drivers that connect to ethernet
> drivers use some similar scheme.
Why? They should be attached to a "real" device, why make any up?
> Should it be some generic virtual bus?
There is a generic virtual bus today.
> This is for a PCI device that plugs into multiple subsystems in the
> kernel, ie it has net driver functionality, rdma functionality, some
> even have SCSI functionality
Sounds like a MFD device, why aren't you using that functionality
instead?
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v6 net-next 1/5] xdp: allow same allocator usage
From: Jesper Dangaard Brouer @ 2019-07-04 12:41 UTC (permalink / raw)
To: Ivan Khoronzhuk
Cc: grygorii.strashko, davem, ast, linux-kernel, linux-omap,
xdp-newbies, ilias.apalodimas, netdev, daniel, jakub.kicinski,
john.fastabend, brouer
In-Reply-To: <20190704102239.GA3406@khorivan>
On Thu, 4 Jul 2019 13:22:40 +0300
Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote:
> On Wed, Jul 03, 2019 at 07:40:13PM +0200, Jesper Dangaard Brouer wrote:
> >On Wed, 3 Jul 2019 13:18:59 +0300
> >Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote:
> >
> >> First of all, it is an absolute requirement that each RX-queue have
> >> their own page_pool object/allocator. And this change is intendant
> >> to handle special case, where a single RX-queue can receive packets
> >> from two different net_devices.
> >>
> >> In order to protect against using same allocator for 2 different rx
> >> queues, add queue_index to xdp_mem_allocator to catch the obvious
> >> mistake where queue_index mismatch, as proposed by Jesper Dangaard
> >> Brouer.
> >>
> >> Adding this on xdp allocator level allows drivers with such dependency
> >> change the allocators w/o modifications.
> >>
> >> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> >> ---
> >> include/net/xdp_priv.h | 2 ++
> >> net/core/xdp.c | 55 ++++++++++++++++++++++++++++++++++++++++++
> >> 2 files changed, 57 insertions(+)
> >>
> >> diff --git a/include/net/xdp_priv.h b/include/net/xdp_priv.h
> >> index 6a8cba6ea79a..9858a4057842 100644
> >> --- a/include/net/xdp_priv.h
> >> +++ b/include/net/xdp_priv.h
> >> @@ -18,6 +18,8 @@ struct xdp_mem_allocator {
> >> struct rcu_head rcu;
> >> struct delayed_work defer_wq;
> >> unsigned long defer_warn;
> >> + unsigned long refcnt;
> >> + u32 queue_index;
> >> };
> >
> >I don't like this approach, because I think we need to extend struct
> >xdp_mem_allocator with a net_device pointer, for doing dev_hold(), to
> >correctly handle lifetime issues. (As I tried to explain previously).
> >This will be much harder after this change, which is why I proposed the
> >other patch.
> My concern comes not from zero also.
> It's partly continuation of not answered questions from here:
> https://lwn.net/ml/netdev/20190625122822.GC6485@khorivan/
>
> "For me it's important to know only if it means that alloc.count is
> freed at first call of __mem_id_disconnect() while shutdown.
> The workqueue for the rest is connected only with ring cache protected
> by ring lock and not supposed that alloc.count can be changed while
> workqueue tries to shutdonwn the pool."
Yes. The alloc.count is only freed on first call. I considered
changing the shutdown API, to have two shutdown calls, where the call
used from the work-queue will not have the loop emptying alloc.count,
but instead have a WARN_ON(alloc.count), as it MUST be empty (once is
code running from work-queue).
> So patch you propose to leave works only because of luck, because fast
> cache is cleared before workqueue is scheduled and no races between two
> workqueues for fast cache later. I'm not really against this patch, but
> I have to try smth better.
It is not "luck". It does the correct thing as we never enter the
while loop in __page_pool_request_shutdown() from a work-queue, but it
is not obvious from the code. The not-so-nice thing is that two
work-queue shutdowns will be racing with each-other, in the multi
netdev use-case, but access to the ptr_ring is safe/locked.
> So, the patch is fine only because of specific of page_pool implementation.
> I'm not sure that in future similar workqueue completion will be lucky for
> another allocator (it easily can happen due to xdp frame can live longer
> than an allocator). Similar problem can happen with other drivers having
> same allocator, that can use zca (potentially can use smth similar),
> af_xdp api allows to switch on it or some other allocators....
>
> But not the essence. The concern about adding smth new to the allocator
> later, like net device, can be solved with a little modification to the patch,
> (despite here can be several more approaches) for instance, like this:
> (by fact it's still the same, when mem_alloc instance per each register call
> but with same void *allocator)
Okay, below you have demonstrated that it is possible to extend later,
although it will make the code (IMHO) "ugly" and more complicated...
So, I guess, I cannot object to this not being extensible.
> diff --git a/include/net/xdp_priv.h b/include/net/xdp_priv.h
> index 6a8cba6ea79a..c7ad0f41e1b0 100644
> --- a/include/net/xdp_priv.h
> +++ b/include/net/xdp_priv.h
> @@ -18,6 +18,8 @@ struct xdp_mem_allocator {
> struct rcu_head rcu;
> struct delayed_work defer_wq;
> unsigned long defer_warn;
> + unsigned long *refcnt;
> + u32 queue_index;
> };
>
> #endif /* __LINUX_NET_XDP_PRIV_H__ */
> diff --git a/net/core/xdp.c b/net/core/xdp.c
> index 829377cc83db..a44e3e4c8307 100644
> --- a/net/core/xdp.c
> +++ b/net/core/xdp.c
> @@ -64,9 +64,37 @@ static const struct rhashtable_params mem_id_rht_params = {
> .obj_cmpfn = xdp_mem_id_cmp,
> };
>
> +static struct xdp_mem_allocator *xdp_allocator_find(void *allocator)
> +{
> + struct xdp_mem_allocator *xae, *xa = NULL;
> + struct rhashtable_iter iter;
> +
> + if (!allocator)
> + return xa;
> +
> + rhashtable_walk_enter(mem_id_ht, &iter);
> + do {
> + rhashtable_walk_start(&iter);
> +
> + while ((xae = rhashtable_walk_next(&iter)) && !IS_ERR(xae)) {
> + if (xae->allocator == allocator) {
> + xa = xae;
> + break;
> + }
> + }
> +
> + rhashtable_walk_stop(&iter);
> +
> + } while (xae == ERR_PTR(-EAGAIN));
> + rhashtable_walk_exit(&iter);
> +
> + return xa;
> +}
> +
> static void __xdp_mem_allocator_rcu_free(struct rcu_head *rcu)
> {
> struct xdp_mem_allocator *xa;
> + void *allocator;
>
> xa = container_of(rcu, struct xdp_mem_allocator, rcu);
>
> @@ -74,15 +102,27 @@ static void __xdp_mem_allocator_rcu_free(struct rcu_head *rcu)
> if (xa->mem.type == MEM_TYPE_PAGE_POOL)
> page_pool_free(xa->page_pool);
>
> - /* Allow this ID to be reused */
> - ida_simple_remove(&mem_id_pool, xa->mem.id);
> + kfree(xa->refcnt);
> + allocator = xa->allocator;
> + while (xa) {
> + xa = xdp_allocator_find(allocator);
> + if (!xa)
> + break;
> +
> + mutex_lock(&mem_id_lock);
> + rhashtable_remove_fast(mem_id_ht, &xa->node, mem_id_rht_params);
> + mutex_unlock(&mem_id_lock);
>
> - /* Poison memory */
> - xa->mem.id = 0xFFFF;
> - xa->mem.type = 0xF0F0;
> - xa->allocator = (void *)0xDEAD9001;
> + /* Allow this ID to be reused */
> + ida_simple_remove(&mem_id_pool, xa->mem.id);
>
> - kfree(xa);
> + /* Poison memory */
> + xa->mem.id = 0xFFFF;
> + xa->mem.type = 0xF0F0;
> + xa->allocator = (void *)0xDEAD9001;
> +
> + kfree(xa);
> + }
> }
>
> static bool __mem_id_disconnect(int id, bool force)
> @@ -98,6 +138,18 @@ static bool __mem_id_disconnect(int id, bool force)
> WARN(1, "Request remove non-existing id(%d), driver bug?", id);
> return true;
> }
> +
> + /* to avoid calling hash lookup twice, decrement refcnt here till it
> + * reaches zero, then it can be called from workqueue afterwards.
> + */
> + if (*xa->refcnt)
> + (*xa->refcnt)--;
> +
> + if (*xa->refcnt) {
> + mutex_unlock(&mem_id_lock);
> + return true;
> + }
> +
> xa->disconnect_cnt++;
>
> /* Detects in-flight packet-pages for page_pool */
> @@ -106,8 +158,7 @@ static bool __mem_id_disconnect(int id, bool force)
>
> trace_mem_disconnect(xa, safe_to_remove, force);
>
> - if ((safe_to_remove || force) &&
> - !rhashtable_remove_fast(mem_id_ht, &xa->node, mem_id_rht_params))
> + if (safe_to_remove || force)
> call_rcu(&xa->rcu, __xdp_mem_allocator_rcu_free);
>
> mutex_unlock(&mem_id_lock);
> @@ -316,6 +367,7 @@ int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
> enum xdp_mem_type type, void *allocator)
> {
> struct xdp_mem_allocator *xdp_alloc;
> + unsigned long *refcnt = NULL;
> gfp_t gfp = GFP_KERNEL;
> int id, errno, ret;
> void *ptr;
> @@ -347,6 +399,19 @@ int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
> }
> }
>
> + mutex_lock(&mem_id_lock);
> + xdp_alloc = xdp_allocator_find(allocator);
> + if (xdp_alloc) {
> + /* One allocator per queue is supposed only */
> + if (xdp_alloc->queue_index != xdp_rxq->queue_index) {
> + mutex_unlock(&mem_id_lock);
> + return -EINVAL;
> + }
> +
> + refcnt = xdp_alloc->refcnt;
> + }
> + mutex_unlock(&mem_id_lock);
> +
> xdp_alloc = kzalloc(sizeof(*xdp_alloc), gfp);
> if (!xdp_alloc)
> return -ENOMEM;
> @@ -360,6 +425,7 @@ int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
> xdp_rxq->mem.id = id;
> xdp_alloc->mem = xdp_rxq->mem;
> xdp_alloc->allocator = allocator;
> + xdp_alloc->queue_index = xdp_rxq->queue_index;
>
> /* Insert allocator into ID lookup table */
> ptr = rhashtable_insert_slow(mem_id_ht, &id, &xdp_alloc->node);
> @@ -370,6 +436,16 @@ int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
> goto err;
> }
>
> + if (!refcnt) {
> + refcnt = kzalloc(sizeof(*xdp_alloc->refcnt), gfp);
> + if (!refcnt) {
> + errno = -ENOMEM;
> + goto err;
> + }
> + }
> +
> + (*refcnt)++;
> + xdp_alloc->refcnt = refcnt;
> mutex_unlock(&mem_id_lock);
>
> trace_mem_connect(xdp_alloc, xdp_rxq);
>
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: [net-next 1/3] ice: Initialize and register platform device to provide RDMA
From: Jason Gunthorpe @ 2019-07-04 12:37 UTC (permalink / raw)
To: Greg KH
Cc: Jeff Kirsher, davem@davemloft.net, dledford@redhat.com,
Tony Nguyen, netdev@vger.kernel.org, linux-rdma@vger.kernel.org,
nhorman@redhat.com, sassmann@redhat.com, poswald@suse.com,
mustafa.ismail@intel.com, shiraz.saleem@intel.com, Dave Ertman,
Andrew Bowers
In-Reply-To: <20190704122950.GA6007@kroah.com>
On Thu, Jul 04, 2019 at 02:29:50PM +0200, Greg KH wrote:
> On Thu, Jul 04, 2019 at 12:16:41PM +0000, Jason Gunthorpe wrote:
> > On Wed, Jul 03, 2019 at 07:12:50PM -0700, Jeff Kirsher wrote:
> > > From: Tony Nguyen <anthony.l.nguyen@intel.com>
> > >
> > > The RDMA block does not advertise on the PCI bus or any other bus.
> > > Thus the ice driver needs to provide access to the RDMA hardware block
> > > via a virtual bus; utilize the platform bus to provide this access.
> > >
> > > This patch initializes the driver to support RDMA as well as creates
> > > and registers a platform device for the RDMA driver to register to. At
> > > this point the driver is fully initialized to register a platform
> > > driver, however, can not yet register as the ops have not been
> > > implemented.
> >
> > I think you need Greg's ack on all this driver stuff - particularly
> > that a platform_device is OK.
>
> A platform_device is almost NEVER ok.
>
> Don't abuse it, make a real device on a real bus. If you don't have a
> real bus and just need to create a device to hang other things off of,
> then use the virtual one, that's what it is there for.
Ideally I'd like to see all the RDMA drivers that connect to ethernet
drivers use some similar scheme.
Should it be some generic virtual bus?
This is for a PCI device that plugs into multiple subsystems in the
kernel, ie it has net driver functionality, rdma functionality, some
even have SCSI functionality
Jason
^ permalink raw reply
* [PATCH] net: ethernet: sun: remove redundant assignment to variable err
From: Colin King @ 2019-07-04 12:36 UTC (permalink / raw)
To: David S . Miller, netdev; +Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
The variable err is being assigned with a value that is never
read and it is being updated in the next statement with a new value.
The assignment is redundant and can be removed.
Addresses-Coverity: ("Unused value")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/net/ethernet/sun/niu.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/net/ethernet/sun/niu.c b/drivers/net/ethernet/sun/niu.c
index 6f99437a6962..0bc5863bffeb 100644
--- a/drivers/net/ethernet/sun/niu.c
+++ b/drivers/net/ethernet/sun/niu.c
@@ -1217,8 +1217,6 @@ static int link_status_1g_rgmii(struct niu *np, int *link_up_p)
spin_lock_irqsave(&np->lock, flags);
- err = -EINVAL;
-
err = mii_read(np, np->phy_addr, MII_BMSR);
if (err < 0)
goto out;
--
2.20.1
^ permalink raw reply related
* Re: [PATCH 2/3] module: Fix up module_notifier return values.
From: Robert Richter @ 2019-07-04 12:34 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Jessica Yu, linux-kernel, jpoimboe, jikos, mbenes, pmladek, ast,
daniel, akpm, Steven Rostedt, Ingo Molnar, Martin KaFai Lau,
Song Liu, Yonghong Song, Mathieu Desnoyers, Paul E. McKenney,
Joel Fernandes (Google), Ard Biesheuvel, Thomas Gleixner,
oprofile-list, netdev, bpf
In-Reply-To: <20190624092109.805742823@infradead.org>
On 24.06.19 11:18:45, Peter Zijlstra wrote:
> While auditing all module notifiers I noticed a whole bunch of fail
> wrt the return value. Notifiers have a 'special' return semantics.
>
> Cc: Robert Richter <rric@kernel.org>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Alexei Starovoitov <ast@kernel.org>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Cc: Martin KaFai Lau <kafai@fb.com>
> Cc: Song Liu <songliubraving@fb.com>
> Cc: Yonghong Song <yhs@fb.com>
> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> Cc: "Paul E. McKenney" <paulmck@linux.ibm.com>
> Cc: "Joel Fernandes (Google)" <joel@joelfernandes.org>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: oprofile-list@lists.sf.net
> Cc: linux-kernel@vger.kernel.org
> Cc: netdev@vger.kernel.org
> Cc: bpf@vger.kernel.org
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
> drivers/oprofile/buffer_sync.c | 4 ++--
> kernel/module.c | 9 +++++----
> kernel/trace/bpf_trace.c | 8 ++++++--
> kernel/trace/trace.c | 2 +-
> kernel/trace/trace_events.c | 2 +-
> kernel/trace/trace_printk.c | 4 ++--
> kernel/tracepoint.c | 2 +-
> 7 files changed, 18 insertions(+), 13 deletions(-)
Reviewed-by: Robert Richter <rric@kernel.org>
^ permalink raw reply
* RE: [for-next V2 10/10] RDMA/core: Provide RDMA DIM support for ULPs
From: Idan Burstein @ 2019-07-04 12:30 UTC (permalink / raw)
To: Sagi Grimberg, Saeed Mahameed, David S. Miller, Doug Ledford,
Jason Gunthorpe
Cc: Leon Romanovsky, Or Gerlitz, Tal Gilboa, netdev@vger.kernel.org,
linux-rdma@vger.kernel.org, Yamin Friedman, Max Gurtovoy
In-Reply-To: <9d26c90c-8e0b-656f-341f-a67251549126@grimberg.me>
The essence of the dynamic in DIM is that it would fit to the workload running on the cores. For user not to trade bandwidth/cqu% and latency with a module parameter they don't know how to config. If DIM consistently hurts latency of latency critical workloads we should debug and fix.
This is where we should go. End goal of no configurate with out of the box performance in terms of both bandwidth/cpu% and latency.
We could make several steps towards this direction if we are not mature enough today but let's define them (e.g. tests on different ulps).
-----Original Message-----
From: linux-rdma-owner@vger.kernel.org <linux-rdma-owner@vger.kernel.org> On Behalf Of Sagi Grimberg
Sent: Tuesday, July 2, 2019 8:37 AM
To: Idan Burstein <idanb@mellanox.com>; Saeed Mahameed <saeedm@mellanox.com>; David S. Miller <davem@davemloft.net>; Doug Ledford <dledford@redhat.com>; Jason Gunthorpe <jgg@mellanox.com>
Cc: Leon Romanovsky <leonro@mellanox.com>; Or Gerlitz <ogerlitz@mellanox.com>; Tal Gilboa <talgi@mellanox.com>; netdev@vger.kernel.org; linux-rdma@vger.kernel.org; Yamin Friedman <yaminf@mellanox.com>; Max Gurtovoy <maxg@mellanox.com>
Subject: Re: [for-next V2 10/10] RDMA/core: Provide RDMA DIM support for ULPs
Hey Idan,
> " Please don't. This is a bad choice to opt it in by default."
>
> I disagree here. I'd prefer Linux to have good out of the box experience (e.g. reach 100G in 4K NVMeOF on Intel servers) with the default parameters. Especially since Yamin have shown it is beneficial / not hurting in terms of performance for variety of use cases. The whole concept of DIM is that it adapts to the workload requirements in terms of bandwidth and latency.
Well, its a Mellanox device driver after all.
But do note that by far, the vast majority of users are not saturating 100G of 4K I/O. The absolute vast majority of users are primarily sensitive to synchronous QD=1 I/O latency, and when the workload is much more dynamic than the synthetic 100%/50%/0% read mix.
As much as I'm a fan (IIRC I was the one giving a first pass at this), the dim default opt-in is not only not beneficial, but potentially harmful to the majority of users out-of-the-box experience.
Given that this is a fresh code with almost no exposure, and that was not tested outside of Yamin running limited performance testing, I think it would be a mistake to add it as a default opt-in, that can come as an incremental stage.
Obviously, I cannot tell what Mellanox should/shouldn't do in its own device driver of course, but I just wanted to emphasize that I think this is a mistake.
> Moreover, net-dim is enabled by default, I don't see why RDMA is different.
Very different animals.
^ permalink raw reply
* Re: [net-next 1/3] ice: Initialize and register platform device to provide RDMA
From: Greg KH @ 2019-07-04 12:29 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Jeff Kirsher, davem@davemloft.net, dledford@redhat.com,
Tony Nguyen, netdev@vger.kernel.org, linux-rdma@vger.kernel.org,
nhorman@redhat.com, sassmann@redhat.com, poswald@suse.com,
mustafa.ismail@intel.com, shiraz.saleem@intel.com, Dave Ertman,
Andrew Bowers
In-Reply-To: <20190704121632.GB3401@mellanox.com>
On Thu, Jul 04, 2019 at 12:16:41PM +0000, Jason Gunthorpe wrote:
> On Wed, Jul 03, 2019 at 07:12:50PM -0700, Jeff Kirsher wrote:
> > From: Tony Nguyen <anthony.l.nguyen@intel.com>
> >
> > The RDMA block does not advertise on the PCI bus or any other bus.
> > Thus the ice driver needs to provide access to the RDMA hardware block
> > via a virtual bus; utilize the platform bus to provide this access.
> >
> > This patch initializes the driver to support RDMA as well as creates
> > and registers a platform device for the RDMA driver to register to. At
> > this point the driver is fully initialized to register a platform
> > driver, however, can not yet register as the ops have not been
> > implemented.
>
> I think you need Greg's ack on all this driver stuff - particularly
> that a platform_device is OK.
A platform_device is almost NEVER ok.
Don't abuse it, make a real device on a real bus. If you don't have a
real bus and just need to create a device to hang other things off of,
then use the virtual one, that's what it is there for.
thanks,
greg k-h
^ permalink raw reply
* [PATCH iproute2 1/1] utils: Fix get_s64() function
From: Kurt Kanzenbach @ 2019-07-04 12:24 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, Kurt Kanzenbach
In-Reply-To: <20190704122427.22256-1-kurt@linutronix.de>
get_s64() uses internally strtoll() to parse the value out of a given
string. strtoll() returns a long long. However, the intermediate variable is
long only which might be 32 bit on some systems. So, fix it.
Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de>
---
lib/utils.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/utils.c b/lib/utils.c
index be0f11b00280..9c3702fd4a04 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -390,7 +390,7 @@ int get_u8(__u8 *val, const char *arg, int base)
int get_s64(__s64 *val, const char *arg, int base)
{
- long res;
+ long long res;
char *ptr;
errno = 0;
--
2.11.0
^ permalink raw reply related
* [PATCH iproute2 0/1] Fix s64 argument parsing
From: Kurt Kanzenbach @ 2019-07-04 12:24 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, Kurt Kanzenbach
Hi,
while using the TAPRIO Qdisc on ARM32 I've noticed that the base_time parameter is
incorrectly configured. The problem is the utility function get_s64() used by
TAPRIO doesn't parse the value correctly.
Thanks,
Kurt
Kurt Kanzenbach (1):
utils: Fix get_s64() function
lib/utils.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--
2.11.0
^ permalink raw reply
* Re: [PATCH ipsec v2] xfrm interface: fix memory leak on creation
From: Nicolas Dichtel @ 2019-07-04 12:23 UTC (permalink / raw)
To: Steffen Klassert
Cc: davem, netdev, Lorenzo Colitti, Benedict Wong, Shannon Nelson,
Antony Antony, Eyal Birger, Julien Floret
In-Reply-To: <20190704102210.GK17989@gauss3.secunet.de>
Le 04/07/2019 à 12:22, Steffen Klassert a écrit :
[snip]
>
> Applied, thanks a lot!
>
I suppose that this patch will be queued for stable trees?
Regards,
Nicolas
^ permalink raw reply
* Re: [PATCH net-next v6 06/15] ethtool: netlink bitset handling
From: Johannes Berg @ 2019-07-04 12:21 UTC (permalink / raw)
To: Michal Kubecek, netdev
Cc: Jiri Pirko, David Miller, Jakub Kicinski, Andrew Lunn,
Florian Fainelli, John Linville, Stephen Hemminger, linux-kernel
In-Reply-To: <20190704121718.GS20101@unicorn.suse.cz>
On Thu, 2019-07-04 at 14:17 +0200, Michal Kubecek wrote:
> On Thu, Jul 04, 2019 at 02:03:02PM +0200, Johannes Berg wrote:
> > On Thu, 2019-07-04 at 13:52 +0200, Michal Kubecek wrote:
> > >
> > > There is still the question if it it should be implemented as a nested
> > > attribute which could look like the current compact form without the
> > > "list" flag (if there is no mask, it's a list). Or an unstructured data
> > > block consisting of u32 bit length
> >
> > You wouldn't really need the length, since the attribute has a length
> > already :-)
>
> It has byte length, not bit length. The bitmaps we are dealing with
> can have any bit length, not necessarily multiples of 8 (or even 32).
Not sure why that matters? You have the mask, so you don't really need
to additionally say that you're only going up to a certain bit?
I mean, say you want to set some bits <=17, why would you need to say
that they're <=17 if you have a
value: 0b00000000'000000xx'xxxxxxxx'xxxxxxxx
mask: 0b00000000'00000011'11111111'11111111
johannes
^ permalink raw reply
* Re: [rdma 14/16] RDMA/irdma: Add ABI definitions
From: Jason Gunthorpe @ 2019-07-04 12:19 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Jeff Kirsher, dledford@redhat.com, davem@davemloft.net,
Mustafa Ismail, linux-rdma@vger.kernel.org,
netdev@vger.kernel.org, nhorman@redhat.com, sassmann@redhat.com,
poswald@suse.com, david.m.ertman@intel.com, Shiraz Saleem
In-Reply-To: <20190704074021.GH4727@mtr-leonro.mtl.com>
On Thu, Jul 04, 2019 at 10:40:21AM +0300, Leon Romanovsky wrote:
> On Wed, Jul 03, 2019 at 07:12:57PM -0700, Jeff Kirsher wrote:
> > From: Mustafa Ismail <mustafa.ismail@intel.com>
> >
> > Add ABI definitions for irdma.
> >
> > Signed-off-by: Mustafa Ismail <mustafa.ismail@intel.com>
> > Signed-off-by: Shiraz Saleem <shiraz.saleem@intel.com>
> > include/uapi/rdma/irdma-abi.h | 130 ++++++++++++++++++++++++++++++++++
> > 1 file changed, 130 insertions(+)
> > create mode 100644 include/uapi/rdma/irdma-abi.h
> >
> > diff --git a/include/uapi/rdma/irdma-abi.h b/include/uapi/rdma/irdma-abi.h
> > new file mode 100644
> > index 000000000000..bdfbda4c829e
> > +++ b/include/uapi/rdma/irdma-abi.h
> > @@ -0,0 +1,130 @@
> > +/* SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause */
> > +/* Copyright (c) 2006 - 2019 Intel Corporation. All rights reserved.
> > + * Copyright (c) 2005 Topspin Communications. All rights reserved.
> > + * Copyright (c) 2005 Cisco Systems. All rights reserved.
> > + * Copyright (c) 2005 Open Grid Computing, Inc. All rights reserved.
> > + */
> > +
> > +#ifndef IRDMA_ABI_H
> > +#define IRDMA_ABI_H
> > +
> > +#include <linux/types.h>
> > +
> > +/* irdma must support legacy GEN_1 i40iw kernel
> > + * and user-space whose last ABI ver is 5
> > + */
> > +#define IRDMA_ABI_VER 6
>
> Can you please elaborate about it more?
> There is no irdma code in RDMA yet, so it makes me wonder why new define
> shouldn't start from 1.
It is because they are ABI compatible with the current user space,
which raises the question why we even have this confusing header
file..
I think this needs to be added after you delete the old driver.
Jason
^ permalink raw reply
* Re: [rdma 1/1] RDMA/irdma: Add Kconfig and Makefile
From: Jason Gunthorpe @ 2019-07-04 12:18 UTC (permalink / raw)
To: Jeff Kirsher
Cc: dledford@redhat.com, davem@davemloft.net, Shiraz Saleem,
linux-rdma@vger.kernel.org, netdev@vger.kernel.org,
nhorman@redhat.com, sassmann@redhat.com, poswald@suse.com,
david.m.ertman@intel.com, mustafa.ismail@intel.com
In-Reply-To: <20190704021259.15489-2-jeffrey.t.kirsher@intel.com>
On Wed, Jul 03, 2019 at 07:12:43PM -0700, Jeff Kirsher wrote:
> From: Shiraz Saleem <shiraz.saleem@intel.com>
>
> Add Kconfig and Makefile to build irdma driver and mark i40iw
> deprecated/obsolete, since the irdma driver is replacing it and supports
> x722 devices.
Patch 1/1? Series looks mangled...
> diff --git a/drivers/infiniband/hw/i40iw/Kconfig b/drivers/infiniband/hw/i40iw/Kconfig
> index d867ef1ac72a..7454b84b74be 100644
> +++ b/drivers/infiniband/hw/i40iw/Kconfig
> @@ -1,8 +1,10 @@
> config INFINIBAND_I40IW
> - tristate "Intel(R) Ethernet X722 iWARP Driver"
> + tristate "Intel(R) Ethernet X722 iWARP Driver (DEPRECATED)"
> depends on INET && I40E
> depends on IPV6 || !IPV6
> depends on PCI
> + depends on !(INFINBAND_IRDMA=y || INFINIBAND_IRDMA=m)
No.. all drivers must be able to build at once. At least add some
COMPILE_TEST in here to enable building.
Jason
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox