* Re: [v3] Bluetooth: btrsi: rework dependencies
From: Kalle Valo @ 2018-03-27 7:12 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Arnd Bergmann, Marcel Holtmann, Johan Hedberg, Johan Hovold,
Amitkumar Karwar, Siva Rebbagondla, linux-bluetooth, linux-kernel,
linux-wireless, netdev
In-Reply-To: <20180315211904.2256817-1-arnd@arndb.de>
Arnd Bergmann <arnd@arndb.de> wrote:
> The linkage between the bluetooth driver and the wireless
> driver is not defined properly, leading to build problems
> such as:
>
> warning: (BT_HCIRSI) selects RSI_COEX which has unmet direct dependencies (NETDEVICES && WLAN && WLAN_VENDOR_RSI && BT_HCIRSI && RSI_91X)
> drivers/net/wireless/rsi/rsi_91x_main.o: In function `rsi_read_pkt':
> (.text+0x205): undefined reference to `rsi_bt_ops'
>
> As the dependency is actually the reverse (RSI_91X uses
> the BT_RSI driver, not the other way round), this changes
> the dependency to match, and enables the bluetooth driver
> from the RSI_COEX symbol.
>
> Fixes: 38aa4da50483 ("Bluetooth: btrsi: add new rsi bluetooth driver")
> Acked-by; Marcel Holtmann <marcel@holtmann.org>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Patch applied to wireless-drivers-next.git, thanks.
255dd5b79d54 Bluetooth: btrsi: rework dependencies
--
https://patchwork.kernel.org/patch/10285795/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [2/4] wireless: Use octal not symbolic permissions
From: Kalle Valo @ 2018-03-27 8:02 UTC (permalink / raw)
To: Joe Perches
Cc: Emmanuel Grumbach, Stanislaw Gruszka, Jiri Slaby, Chi-Hsien Lin,
Helmut Schaa, Kalle Valo, Luca Coelho, Solomon Peachy,
Christian Lamparter, wcn36xx, brcm80211-dev-list, Jakub Kicinski,
Nick Kossifidis, Stanislav Yakovlev, Arend van Spriel,
Johannes Berg, Intel Linux Wireless, Hante Meuleman,
linux-mediatek, Wright Feng, Matthias Brugger, linux-arm-kernel
In-Reply-To: <a368e1c4878619172f3da4849cb211b4dfad0a58.1521845237.git.joe@perches.com>
Joe Perches <joe@perches.com> wrote:
> Prefer the direct use of octal for permissions.
>
> Done with checkpatch -f --types=SYMBOLIC_PERMS --fix-inplace
> and some typing.
>
> Miscellanea:
>
> o Whitespace neatening around these conversions.
>
> Signed-off-by: Joe Perches <joe@perches.com>
Patch applied to wireless-drivers-next.git, thanks.
2ef00c53049b wireless: Use octal not symbolic permissions
--
https://patchwork.kernel.org/patch/10305719/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [2/4] wireless: Use octal not symbolic permissions
From: Kalle Valo @ 2018-03-27 8:02 UTC (permalink / raw)
To: Joe Perches
Cc: Jiri Slaby, Nick Kossifidis, Luis R. Rodriguez, Kalle Valo,
QCA ath9k Development, Christian Lamparter, Eugene Krasnikov,
Stanislav Yakovlev, Stanislaw Gruszka, Johannes Berg,
Emmanuel Grumbach, Luca Coelho, Intel Linux Wireless,
Jakub Kicinski, Helmut Schaa, Solomon Peachy, Matthias Brugger,
Arend van Spriel
In-Reply-To: <a368e1c4878619172f3da4849cb211b4dfad0a58.1521845237.git.joe@perches.com>
Joe Perches <joe@perches.com> wrote:
> Prefer the direct use of octal for permissions.
>
> Done with checkpatch -f --types=SYMBOLIC_PERMS --fix-inplace
> and some typing.
>
> Miscellanea:
>
> o Whitespace neatening around these conversions.
>
> Signed-off-by: Joe Perches <joe@perches.com>
Patch applied to wireless-drivers-next.git, thanks.
2ef00c53049b wireless: Use octal not symbolic permissions
--
https://patchwork.kernel.org/patch/10305719/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [v2] rsi: Remove stack VLA usage
From: Kalle Valo @ 2018-03-27 8:04 UTC (permalink / raw)
To: Tobin C. Harding
Cc: Tobin C. Harding, kernel-hardening, linux-kernel, netdev,
linux-wireless, Tycho Andersen, Kees Cook, Larry Finger
In-Reply-To: <1521081085-16404-1-git-send-email-me@tobin.cc>
"Tobin C. Harding" <me@tobin.cc> wrote:
> The use of stack Variable Length Arrays needs to be avoided, as they
> can be a vector for stack exhaustion, which can be both a runtime bug
> (kernel Oops) or a security flaw (overwriting memory beyond the
> stack). Also, in general, as code evolves it is easy to lose track of
> how big a VLA can get. Thus, we can end up having runtime failures
> that are hard to debug. As part of the directive[1] to remove all VLAs
> from the kernel, and build with -Wvla.
>
> Currently rsi code uses a VLA based on a function argument to
> `rsi_sdio_load_data_master_write()`. The function call chain is
>
> Both these functions
>
> rsi_sdio_reinit_device()
> rsi_probe()
>
> start the call chain:
>
> rsi_hal_device_init()
> rsi_load_fw()
> auto_fw_upgrade()
> ping_pong_write()
> rsi_sdio_load_data_master_write()
>
> [Without familiarity with the code] it appears that none of the 4 locks
>
> mutex
> rx_mutex
> tx_mutex
> tx_bus_mutex
>
> are held when `rsi_sdio_load_data_master_write()` is called. It is therefore
> safe to use kmalloc with GFP_KERNEL.
>
> We can avoid using the VLA by using `kmalloc()` and free'ing the memory on all
> exit paths.
>
> Change buffer from 'u8 array' to 'u8 *'. Call `kmalloc()` to allocate memory for
> the buffer. Using goto statement to call `kfree()` on all return paths.
>
> It can be expected that this patch will result in a small increase in overhead
> due to the use of `kmalloc()` however this code is only called on initialization
> (and re-initialization) so this overhead should not degrade performance.
>
> [1] https://lkml.org/lkml/2018/3/7/621
>
> Signed-off-by: Tobin C. Harding <me@tobin.cc>
Patch applied to wireless-drivers-next.git, thanks.
44f98a9332e4 rsi: Remove stack VLA usage
--
https://patchwork.kernel.org/patch/10283841/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [v2] rsi: Remove stack VLA usage
From: Kalle Valo @ 2018-03-27 8:04 UTC (permalink / raw)
To: Tobin C. Harding
Cc: Tobin C. Harding, kernel-hardening, linux-kernel, netdev,
linux-wireless, Tycho Andersen, Kees Cook, Larry Finger
In-Reply-To: <1521081085-16404-1-git-send-email-me@tobin.cc>
"Tobin C. Harding" <me@tobin.cc> wrote:
> The use of stack Variable Length Arrays needs to be avoided, as they
> can be a vector for stack exhaustion, which can be both a runtime bug
> (kernel Oops) or a security flaw (overwriting memory beyond the
> stack). Also, in general, as code evolves it is easy to lose track of
> how big a VLA can get. Thus, we can end up having runtime failures
> that are hard to debug. As part of the directive[1] to remove all VLAs
> from the kernel, and build with -Wvla.
>
> Currently rsi code uses a VLA based on a function argument to
> `rsi_sdio_load_data_master_write()`. The function call chain is
>
> Both these functions
>
> rsi_sdio_reinit_device()
> rsi_probe()
>
> start the call chain:
>
> rsi_hal_device_init()
> rsi_load_fw()
> auto_fw_upgrade()
> ping_pong_write()
> rsi_sdio_load_data_master_write()
>
> [Without familiarity with the code] it appears that none of the 4 locks
>
> mutex
> rx_mutex
> tx_mutex
> tx_bus_mutex
>
> are held when `rsi_sdio_load_data_master_write()` is called. It is therefore
> safe to use kmalloc with GFP_KERNEL.
>
> We can avoid using the VLA by using `kmalloc()` and free'ing the memory on all
> exit paths.
>
> Change buffer from 'u8 array' to 'u8 *'. Call `kmalloc()` to allocate memory for
> the buffer. Using goto statement to call `kfree()` on all return paths.
>
> It can be expected that this patch will result in a small increase in overhead
> due to the use of `kmalloc()` however this code is only called on initialization
> (and re-initialization) so this overhead should not degrade performance.
>
> [1] https://lkml.org/lkml/2018/3/7/621
>
> Signed-off-by: Tobin C. Harding <me@tobin.cc>
Patch applied to wireless-drivers-next.git, thanks.
44f98a9332e4 rsi: Remove stack VLA usage
--
https://patchwork.kernel.org/patch/10283841/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* RE: [RFC PATCH v2] net: phy: Added device tree binding for dev-addr and dev-addr code check-up
From: Vicenţiu Galanopulo @ 2018-03-27 8:10 UTC (permalink / raw)
To: Rob Herring
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
mark.rutland@arm.com, davem@davemloft.net, marcel@holtmann.org,
devicetree@vger.kernel.org, Madalin-cristian Bucur,
Alexandru Marginean
In-Reply-To: <20180326222509.ppt5aqzqdarhm44d@rob-hp-laptop>
> -----Original Message-----
> From: Rob Herring [mailto:robh@kernel.org]
> Sent: Tuesday, March 27, 2018 1:25 AM
> To: Vicenţiu Galanopulo <vicentiu.galanopulo@nxp.com>
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> mark.rutland@arm.com; davem@davemloft.net; marcel@holtmann.org;
> devicetree@vger.kernel.org; Madalin-cristian Bucur <madalin.bucur@nxp.com>;
> Alexandru Marginean <alexandru.marginean@nxp.com>
> Subject: Re: [RFC PATCH v2] net: phy: Added device tree binding for dev-addr
> and dev-addr code check-up
>
> On Fri, Mar 23, 2018 at 10:05:22AM -0500, Vicentiu Galanopulo wrote:
> > Reason for this patch is that the Inphi PHY has a vendor specific
> > address space for accessing the
> > C45 MDIO registers - starting from 0x1e.
> >
> > A search of the dev-addr property is done in of_mdiobus_register.
> > If the property is found in the PHY node,
> > of_mdiobus_register_static_phy is called. This is a wrapper function
> > for of_mdiobus_register_phy which finds the device in package based on
> > dev-addr and fills devices_addrs:
> > devices_addrs is a new field added to phy_c45_device_ids.
> > This new field will store the dev-addr property on the same index
> > where the device in package has been found.
> > In order to have dev-addr in get_phy_c45_ids(), mdio_c45_ids is passed
> > from of_mdio.c to phy_device.c as an external variable.
> > In get_phy_device a copy of the mdio_c45_ids is done over the local
> > c45_ids (wich are empty). After the copying, the c45_ids will also
> > contain the static device found from dev-addr.
> > Having dev-addr stored in devices_addrs, in get_phy_c45_ids(), when
> > probing the identifiers, dev-addr can be extracted from devices_addrs
> > and probed if devices_addrs[current_identifier] is not 0.
> > This way changing the kernel API is avoided completely.
> >
> > As a plus to this patch, num_ids in get_phy_c45_ids, has the value 8
> > (ARRAY_SIZE(c45_ids->device_ids)),
> > but the u32 *devs can store 32 devices in the bitfield.
> > If a device is stored in *devs, in bits 32 to 9, it will not be found.
> > This is the reason for changing in phy.h, the size of device_ids
> > array.
> >
> > Signed-off-by: Vicentiu Galanopulo <vicentiu.galanopulo@nxp.com>
> > ---
> > Documentation/devicetree/bindings/net/phy.txt | 6 ++
>
> Please split bindings to separate patch.
Thanks Rob for your comments. I was considering doing that after I reach a more stable, non-RFC version of the patch. I also added a v3, before
your comments, I think... so in the next one, v4, I will split the binding to a separate patch.
>
> > drivers/net/phy/phy_device.c | 22 +++++-
> > drivers/of/of_mdio.c | 98 ++++++++++++++++++++++++++-
> > include/linux/phy.h | 5 +-
> > 4 files changed, 125 insertions(+), 6 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/net/phy.txt
> > b/Documentation/devicetree/bindings/net/phy.txt
> > index d2169a5..82692e2 100644
> > --- a/Documentation/devicetree/bindings/net/phy.txt
> > +++ b/Documentation/devicetree/bindings/net/phy.txt
> > @@ -61,6 +61,11 @@ Optional Properties:
> > - reset-deassert-us: Delay after the reset was deasserted in microseconds.
> > If this property is missing the delay will be skipped.
> >
> > +- dev-addr: If set, it indicates the device address of the PHY to be
> > +used
> > + when accessing the C45 PHY registers over MDIO. It is used for
> > +vendor specific
> > + register space addresses that do no conform to standard address for
> > +the MDIO
> > + registers (e.g. MMD30)
>
> This is a 2nd MDIO address, right? Can't you just append this to reg property?
>
> Rob
Yes, it is a 2nd MDIO address which is coming from the PHY vendor. This address cannot be obtained by querying the MDIO bus, it's specified in the PHY datasheet. The current kernel implementation was relying on the fact that this address is in the decimal 0 to 7 range. That worked for the PHYs which already have a kernel driver, but for the new Inphi PHY, which I'm trying to add, it didn't.
If I were to append the dev-addr address to the reg property, I would have to fit two 32bit variable into a single one... in my opinion the code is clearer having the two addresses as distinct variables.... And so far, the comments from Andrew or Florian didn't go in this direction..
Vicentiu
^ permalink raw reply
* Re: [net-next 03/15] net/mlx5e: PFC stall prevention support
From: Gal Pressman @ 2018-03-27 8:38 UTC (permalink / raw)
To: Andrew Lunn; +Cc: Saeed Mahameed, David S. Miller, netdev, Inbar Karmy
In-Reply-To: <20180325161815.GB12820@lunn.ch>
On 25-Mar-18 19:18, Andrew Lunn wrote:
>>> Shouldn't you map a value of MLX5E_PFC_PREVEN_AUTO_TOUT_MSEC back to
>>> PFC_STORM_PREVENTION_AUTO?
>>
>> We discussed this point internally, mapping MLX5E_PFC_PREVEN_AUTO_TOUT_MSEC (100) to
>> PFC_STORM_PREVENTION_AUTO might cause confusion when the user explicitly asks for 100msec timeout
>> and gets auto in his following query.
>> Also, this way the "auto" timeout is visible to the user, which might help him get an initial
>> clue of which values are recommended.
>
> Yes, this is a fair point, which is why i asked the question. Either
> way, it can cause confusion. 'I configured it to auto, but it always
> returns 100, not auto.'
>
> Whatever is decided, it should be consistent across drivers. So please
> add some documentation to the ethtool header file about what is
> expected.
We didn't want to limit other drivers implementation, but I agree that
consistency across drivers is important in this case.
We will find a proper place to document it.
>
> Andrew
>
^ permalink raw reply
* Re: [PATCH bpf-next] bpf: sockmap: initialize sg table entries properly
From: Prashant Bhole @ 2018-03-27 8:41 UTC (permalink / raw)
To: John Fastabend
Cc: Daniel Borkmann, Alexei Starovoitov, David S . Miller, netdev
In-Reply-To: <dd0531c0-9216-89db-6752-213401392705@gmail.com>
On 3/27/2018 12:15 PM, John Fastabend wrote:
> On 03/25/2018 11:54 PM, Prashant Bhole wrote:
>> When CONFIG_DEBUG_SG is set, sg->sg_magic is initialized to SG_MAGIC,
>> when sg table is initialized using sg_init_table(). Magic is checked
>> while navigating the scatterlist. We hit BUG_ON when magic check is
>> failed.
>>
>> Fixed following things:
>> - Initialization of sg table in bpf_tcp_sendpage() was missing,
>> initialized it using sg_init_table()
>>
>> - bpf_tcp_sendmsg() initializes sg table using sg_init_table() before
>> entering the loop, but further consumed sg entries are initialized
>> using memset. Fixed it by replacing memset with sg_init_table() in
>> function bpf_tcp_push()
>>
>> Signed-off-by: Prashant Bhole <bhole_prashant_q7@lab.ntt.co.jp>
>> ---
>> kernel/bpf/sockmap.c | 11 +++++++----
>> 1 file changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c
>> index 69c5bccabd22..8a848a99d768 100644
>> --- a/kernel/bpf/sockmap.c
>> +++ b/kernel/bpf/sockmap.c
>> @@ -312,7 +312,7 @@ static int bpf_tcp_push(struct sock *sk, int apply_bytes,
>> md->sg_start++;
>> if (md->sg_start == MAX_SKB_FRAGS)
>> md->sg_start = 0;
>> - memset(sg, 0, sizeof(*sg));
>> + sg_init_table(sg, 1);
>
> Looks OK here.
>
>>
>> if (md->sg_start == md->sg_end)
>> break;
>> @@ -763,10 +763,14 @@ static int bpf_tcp_sendpage(struct sock *sk, struct page *page,
>>
>> lock_sock(sk);
>>
>> - if (psock->cork_bytes)
>> + if (psock->cork_bytes) {
>> m = psock->cork;
>> - else
>> + sg = &m->sg_data[m->sg_end];
>> + } else {
>> m = &md;
>> + sg = m->sg_data;
>> + sg_init_table(sg, MAX_SKB_FRAGS);
>
> sg_init_table() does an unnecessary memset() though. We
> probably either want a new scatterlist API or just open
> code this,
>
> #ifdef CONFIG_DEBUG_SG
> {
> unsigned int i;
> for (i = 0; i < nents; i++)
> sgl[i].sg_magic = SG_MAGIC;
> }
Similar sg_init_table() is present in bpf_tcp_sendmsg().
I agree that it causes unnecessary memset, but I don't agree with open
coded fix.
I am still with V1. Thanks.
-Prashant
^ permalink raw reply
* [PATCH net 1/1] net/smc: use announced length in sock_recvmsg()
From: Ursula Braun @ 2018-03-27 8:43 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl, ubraun
In-Reply-To: <20180327084350.92847-1-ubraun@linux.vnet.ibm.com>
Not every CLC proposal message needs the maximum buffer length.
Due to the MSG_WAITALL flag, it is important to use the peeked
real length when receiving the message.
Fixes: d63d271ce2b5ce ("smc: switch to sock_recvmsg()")
Signed-off-by: Ursula Braun <ubraun@linux.vnet.ibm.com>
---
net/smc/smc_clc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/smc/smc_clc.c b/net/smc/smc_clc.c
index 8ac51583a063..15c213250606 100644
--- a/net/smc/smc_clc.c
+++ b/net/smc/smc_clc.c
@@ -133,7 +133,7 @@ int smc_clc_wait_msg(struct smc_sock *smc, void *buf, int buflen,
/* receive the complete CLC message */
memset(&msg, 0, sizeof(struct msghdr));
- iov_iter_kvec(&msg.msg_iter, READ | ITER_KVEC, &vec, 1, buflen);
+ iov_iter_kvec(&msg.msg_iter, READ | ITER_KVEC, &vec, 1, datlen);
krflags = MSG_WAITALL;
smc->clcsock->sk->sk_rcvtimeo = CLC_WAIT_TIME;
len = sock_recvmsg(smc->clcsock, &msg, krflags);
--
2.13.5
^ permalink raw reply related
* [PATCH net 0/1] net/smc: fix clc problem
From: Ursula Braun @ 2018-03-27 8:43 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl, ubraun
Dave,
the conversion from kernel_recvmsg() to sock_recvmsg() introduced a
problem for SMC. This patch fixes it.
Thanks, Ursula
Ursula Braun (1):
net/smc: use announced length in sock_recvmsg()
net/smc/smc_clc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--
2.13.5
^ permalink raw reply
* Re: [PATCH net-next nfs 1/6] net: Convert rpcsec_gss_net_ops
From: Kirill Tkhai @ 2018-03-27 8:52 UTC (permalink / raw)
To: J. Bruce Fields, Anna Schumaker
Cc: davem, trond.myklebust, jlayton, dhowells, keescook, dwindsor,
ishkamiel, elena.reshetova, linux-nfs, linux-afs, netdev
In-Reply-To: <20180326183659.GA25744@fieldses.org>
On 26.03.2018 21:36, J. Bruce Fields wrote:
> On Fri, Mar 23, 2018 at 02:53:34PM -0400, Anna Schumaker wrote:
>>
>>
>> On 03/13/2018 06:49 AM, Kirill Tkhai wrote:
>>> These pernet_operations initialize and destroy sunrpc_net_id refered
>>> per-net items. Only used global list is cache_list, and accesses
>>> already serialized.
>>>
>>> sunrpc_destroy_cache_detail() check for list_empty() without
>>> cache_list_lock, but when it's called from
>>> unregister_pernet_subsys(), there can't be callers in parallel, so
>>> we won't miss list_empty() in this case.
>>>
>>> Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
>>
>> It might make sense to take these and the other NFS patches through
>> the net tree, since the pernet_operations don't yet have the async
>> field in my tree (and I therefore can't compile once these are
>> applied).
>
> Ditto for the nfsd patch, so, for what it's worth:
>
> Acked-by: J. Bruce Fields <bfields@redhat.com>
>
> for that patch.--b.
Thanks, Bruce.
Kirill
^ permalink raw reply
* Re: [PATCH 4/4] selftests/bpf: fix compiling errors
From: Daniel Borkmann @ 2018-03-27 8:52 UTC (permalink / raw)
To: Du, Changbin, Alexei Starovoitov
Cc: shuah, linux-kselftest, linux-kernel, netdev
In-Reply-To: <20180327030613.hkp7hox3g4prnbsg@intel.com>
On 03/27/2018 05:06 AM, Du, Changbin wrote:
> On Mon, Mar 26, 2018 at 08:02:30PM -0700, Alexei Starovoitov wrote:
>> On Tue, Mar 27, 2018 at 10:20:10AM +0800, Du, Changbin wrote:
>>> On Mon, Mar 26, 2018 at 07:55:13AM -0700, Alexei Starovoitov wrote:
>>>> On Mon, Mar 26, 2018 at 05:23:28PM +0800, changbin.du@intel.com wrote:
>>>>> Signed-off-by: Changbin Du <changbin.du@intel.com>
>>>>> ---
>>>>> tools/testing/selftests/bpf/Makefile | 5 +++--
>>>>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
>>>>> index 5c43c18..dc0fdc8 100644
>>>>> --- a/tools/testing/selftests/bpf/Makefile
>>>>> +++ b/tools/testing/selftests/bpf/Makefile
>>>>> @@ -10,7 +10,8 @@ ifneq ($(wildcard $(GENHDR)),)
>>>>> GENFLAGS := -DHAVE_GENHDR
>>>>> endif
>>>>>
>>>>> -CFLAGS += -Wall -O2 -I$(APIDIR) -I$(LIBDIR) -I$(GENDIR) $(GENFLAGS) -I../../../include
>>>>> +CFLAGS += -Wall -O2 -I$(APIDIR) -I$(LIBDIR) -I$(GENDIR) $(GENFLAGS) \
>>>>> + -I../../../include -I../../../../usr/include
>>>>> LDLIBS += -lcap -lelf -lrt -lpthread
>>>>>
>>>>> # Order correspond to 'make run_tests' order
>>>>> @@ -62,7 +63,7 @@ else
>>>>> CPU ?= generic
>>>>> endif
>>>>>
>>>>> -CLANG_FLAGS = -I. -I./include/uapi -I../../../include/uapi \
>>>>> +CLANG_FLAGS = -I. -I./include/uapi -I../../../include/uapi -I../../../../usr/include \
>>>>> -Wno-compare-distinct-pointer-types
>>>>
>>>> Nack.
>>>> I suspect that will break the build for everyone else who's doing it in the directory
>>>> itself instead of the outer one.
>>>
>>> This one? But I didn't see any problem.
>>
>> because the build was lucky and additional path ../../../../usr/include didn't point
>> to a valid dir?
Agree.
> I am sorry but I don't understand why you mean *lucky*. Of cause, the path is valid.
The problem is that this suddenly requires users to do a 'make headers_install' in
order to populate usr/include/ directory in the first place. While it's annoying
enough for BPF samples where this is needed, I absolutely don't want to introduce
this for BPF kselftests. It's the wrong approach. Besides, in tools infra, there is
a tools/arch/*/include/uapi/asm/bitsperlong.h header copy already, so we really need
to use that instead. Please adapt your patch accordingly and respin. Please also Cc
us and netdev@vger.kernel.org for BPF kselftests changes.
Thanks,
Daniel
^ permalink raw reply
* Re: [next] rsi: remove redundant duplicate assignment of buffer_size
From: Kalle Valo @ 2018-03-27 8:53 UTC (permalink / raw)
To: Colin Ian King
Cc: Amitkumar Karwar, Prameela Rani Garnepudi, linux-wireless, netdev,
kernel-janitors, linux-kernel
In-Reply-To: <20180314141239.8321-1-colin.king@canonical.com>
Colin Ian King <colin.king@canonical.com> wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Variable buffer_size is re-assigned the same value, this duplicated
> assignment is redundant and can be removed.
>
> Cleans up clang warning:
> drivers/net/wireless/rsi/rsi_91x_usb.c:140:4: warning: Value stored
> to 'buffer_size' is never read
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Patch applied to wireless-drivers-next.git, thanks.
a31f9314114c rsi: remove redundant duplicate assignment of buffer_size
--
https://patchwork.kernel.org/patch/10282247/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH 4/4] selftests/bpf: fix compiling errors
From: Du, Changbin @ 2018-03-27 9:00 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Du, Changbin, Alexei Starovoitov, shuah, linux-kselftest,
linux-kernel, netdev
In-Reply-To: <2dd7c04a-eb10-e730-148e-3b04a18e9b44@iogearbox.net>
On Tue, Mar 27, 2018 at 10:52:57AM +0200, Daniel Borkmann wrote:
> On 03/27/2018 05:06 AM, Du, Changbin wrote:
> > On Mon, Mar 26, 2018 at 08:02:30PM -0700, Alexei Starovoitov wrote:
> >> On Tue, Mar 27, 2018 at 10:20:10AM +0800, Du, Changbin wrote:
> >>> On Mon, Mar 26, 2018 at 07:55:13AM -0700, Alexei Starovoitov wrote:
> >>>> On Mon, Mar 26, 2018 at 05:23:28PM +0800, changbin.du@intel.com wrote:
> >>>>> Signed-off-by: Changbin Du <changbin.du@intel.com>
> >>>>> ---
> >>>>> tools/testing/selftests/bpf/Makefile | 5 +++--
> >>>>> 1 file changed, 3 insertions(+), 2 deletions(-)
> >>>>>
> >>>>> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> >>>>> index 5c43c18..dc0fdc8 100644
> >>>>> --- a/tools/testing/selftests/bpf/Makefile
> >>>>> +++ b/tools/testing/selftests/bpf/Makefile
> >>>>> @@ -10,7 +10,8 @@ ifneq ($(wildcard $(GENHDR)),)
> >>>>> GENFLAGS := -DHAVE_GENHDR
> >>>>> endif
> >>>>>
> >>>>> -CFLAGS += -Wall -O2 -I$(APIDIR) -I$(LIBDIR) -I$(GENDIR) $(GENFLAGS) -I../../../include
> >>>>> +CFLAGS += -Wall -O2 -I$(APIDIR) -I$(LIBDIR) -I$(GENDIR) $(GENFLAGS) \
> >>>>> + -I../../../include -I../../../../usr/include
> >>>>> LDLIBS += -lcap -lelf -lrt -lpthread
> >>>>>
> >>>>> # Order correspond to 'make run_tests' order
> >>>>> @@ -62,7 +63,7 @@ else
> >>>>> CPU ?= generic
> >>>>> endif
> >>>>>
> >>>>> -CLANG_FLAGS = -I. -I./include/uapi -I../../../include/uapi \
> >>>>> +CLANG_FLAGS = -I. -I./include/uapi -I../../../include/uapi -I../../../../usr/include \
> >>>>> -Wno-compare-distinct-pointer-types
> >>>>
> >>>> Nack.
> >>>> I suspect that will break the build for everyone else who's doing it in the directory
> >>>> itself instead of the outer one.
> >>>
> >>> This one? But I didn't see any problem.
> >>
> >> because the build was lucky and additional path ../../../../usr/include didn't point
> >> to a valid dir?
>
> Agree.
>
> > I am sorry but I don't understand why you mean *lucky*. Of cause, the path is valid.
>
> The problem is that this suddenly requires users to do a 'make headers_install' in
> order to populate usr/include/ directory in the first place. While it's annoying
> enough for BPF samples where this is needed, I absolutely don't want to introduce
> this for BPF kselftests. It's the wrong approach. Besides, in tools infra, there is
> a tools/arch/*/include/uapi/asm/bitsperlong.h header copy already, so we really need
> to use that instead. Please adapt your patch accordingly and respin. Please also Cc
> us and netdev@vger.kernel.org for BPF kselftests changes.
>
> Thanks,
> Daniel
Thanks for the explanation. So we expect that tools/arch/*/include is in the searching list, right?
The corrent makefile seems not. How do you get this built?
changbin@gvt-dell-host:~/work/linux/tools/testing/selftests/bpf$ make -p
[...]
clang -I. -I./include/uapi -I../../../include/uapi -Wno-compare-distinct-pointer-types \
-O2 -target bpf -emit-llvm -c test_pkt_access.c -o - | \
llc -march=bpf -mcpu=generic -filetype=obj -o /home/changbin/work/linux/tools/testing/selftests/bpf/test_pkt_access.o
In file included from test_pkt_access.c:9:
In file included from ../../../include/uapi/linux/bpf.h:11:
In file included from ./include/uapi/linux/types.h:5:
/usr/include/asm-generic/int-ll64.h:11:10: fatal error: 'asm/bitsperlong.h' file not found
#include <asm/bitsperlong.h>
--
Thanks,
Changbin Du
^ permalink raw reply
* Re: rtlwifi: rtl8821ae: fix spelling mistake: "Aboslute" -> "Absolute"
From: Kalle Valo @ 2018-03-27 9:03 UTC (permalink / raw)
To: Colin Ian King
Cc: Ping-Ke Shih, Tsang-Shian Lin, Larry Finger, linux-wireless,
netdev, kernel-janitors, linux-kernel
In-Reply-To: <20180319104054.32564-1-colin.king@canonical.com>
Colin Ian King <colin.king@canonical.com> wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Trivial fix to spelling mistake in RT_TRACE message text.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> Acked-by: Larry Finger <Larry.Finger@lwfinger.net?
Patch applied to wireless-drivers-next.git, thanks.
e153292ae4d1 rtlwifi: rtl8821ae: fix spelling mistake: "Aboslute" -> "Absolute"
--
https://patchwork.kernel.org/patch/10292111/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH bpf-next] bpf: sockmap: initialize sg table entries properly
From: Daniel Borkmann @ 2018-03-27 9:05 UTC (permalink / raw)
To: Prashant Bhole, John Fastabend
Cc: Alexei Starovoitov, David S . Miller, netdev
In-Reply-To: <87c1e6e0-913f-2297-986f-f6b70ce1e485@lab.ntt.co.jp>
On 03/27/2018 10:41 AM, Prashant Bhole wrote:
> On 3/27/2018 12:15 PM, John Fastabend wrote:
>> On 03/25/2018 11:54 PM, Prashant Bhole wrote:
>>> When CONFIG_DEBUG_SG is set, sg->sg_magic is initialized to SG_MAGIC,
>>> when sg table is initialized using sg_init_table(). Magic is checked
>>> while navigating the scatterlist. We hit BUG_ON when magic check is
>>> failed.
>>>
>>> Fixed following things:
>>> - Initialization of sg table in bpf_tcp_sendpage() was missing,
>>> initialized it using sg_init_table()
>>>
>>> - bpf_tcp_sendmsg() initializes sg table using sg_init_table() before
>>> entering the loop, but further consumed sg entries are initialized
>>> using memset. Fixed it by replacing memset with sg_init_table() in
>>> function bpf_tcp_push()
>>>
>>> Signed-off-by: Prashant Bhole <bhole_prashant_q7@lab.ntt.co.jp>
>>> ---
>>> kernel/bpf/sockmap.c | 11 +++++++----
>>> 1 file changed, 7 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c
>>> index 69c5bccabd22..8a848a99d768 100644
>>> --- a/kernel/bpf/sockmap.c
>>> +++ b/kernel/bpf/sockmap.c
>>> @@ -312,7 +312,7 @@ static int bpf_tcp_push(struct sock *sk, int apply_bytes,
>>> md->sg_start++;
>>> if (md->sg_start == MAX_SKB_FRAGS)
>>> md->sg_start = 0;
>>> - memset(sg, 0, sizeof(*sg));
>>> + sg_init_table(sg, 1);
>>
>> Looks OK here.
>>
>>> if (md->sg_start == md->sg_end)
>>> break;
>>> @@ -763,10 +763,14 @@ static int bpf_tcp_sendpage(struct sock *sk, struct page *page,
>>> lock_sock(sk);
>>> - if (psock->cork_bytes)
>>> + if (psock->cork_bytes) {
>>> m = psock->cork;
>>> - else
>>> + sg = &m->sg_data[m->sg_end];
>>> + } else {
>>> m = &md;
>>> + sg = m->sg_data;
>>> + sg_init_table(sg, MAX_SKB_FRAGS);
>>
>> sg_init_table() does an unnecessary memset() though. We
>> probably either want a new scatterlist API or just open
>> code this,
>>
>> #ifdef CONFIG_DEBUG_SG
>> {
>> unsigned int i;
>> for (i = 0; i < nents; i++)
>> sgl[i].sg_magic = SG_MAGIC;
>> }
>
> Similar sg_init_table() is present in bpf_tcp_sendmsg().
> I agree that it causes unnecessary memset, but I don't agree with open coded fix.
But then lets fix is properly and add a static inline helper to the
include/linux/scatterlist.h header like ...
static inline void sg_init_debug_marker(struct scatterlist *sgl,
unsigned int nents)
{
#ifdef CONFIG_DEBUG_SG
unsigned int i;
for (i = 0; i < nents; i++)
sgl[i].sg_magic = SG_MAGIC;
#endif
}
... and reuse it in all the places that would otherwise open-code this,
as well as sg_init_table():
void sg_init_table(struct scatterlist *sgl, unsigned int nents)
{
memset(sgl, 0, sizeof(*sgl) * nents);
sg_init_debug_marker(sgl, nents);
sg_mark_end(&sgl[nents - 1]);
}
This would be a lot cleaner than having this duplicated in various places.
Thanks,
Daniel
^ permalink raw reply
* [PATCH] vhost-net: add time limitation for tx polling
From: haibinzhang(张海斌) @ 2018-03-27 9:12 UTC (permalink / raw)
To: mst@redhat.com, jasowang@redhat.com, kvm@vger.kernel.org,
virtualization@lists.linux-foundation.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
handle_tx() will delay rx for a long time when busy tx polling udp packets
with short length(ie: 1byte udp payload), because setting VHOST_NET_WEIGHT
takes into account only sent-bytes but no time. It's not fair for handle_rx(),
so needs to limit max time of tx polling.
Signed-off-by: Haibin Zhang <haibinzhang@tencent.com>
---
drivers/vhost/net.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 8139bc70ad7d..dc9218a3a75b 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -473,6 +473,7 @@ static void handle_tx(struct vhost_net *net)
struct socket *sock;
struct vhost_net_ubuf_ref *uninitialized_var(ubufs);
bool zcopy, zcopy_used;
+ unsigned long start = jiffies;
mutex_lock(&vq->mutex);
sock = vq->private_data;
@@ -580,7 +581,7 @@ static void handle_tx(struct vhost_net *net)
else
vhost_zerocopy_signal_used(net, vq);
vhost_net_tx_packet(net);
- if (unlikely(total_len >= VHOST_NET_WEIGHT)) {
+ if (unlikely(total_len >= VHOST_NET_WEIGHT) || unlikely(jiffies - start >= 1)) {
vhost_poll_queue(&vq->poll);
break;
}
--
2.12.3
^ permalink raw reply related
* [PATCH net] lan78xx: Crash in lan78xx_writ_reg (Workqueue: events lan78xx_deferred_multicast_write)
From: Raghuram Chary J @ 2018-03-27 9:21 UTC (permalink / raw)
To: davem; +Cc: netdev, unglinuxdriver, woojung.huh, raghuramchary.jallipalli
Description:
Crash was reported with syzkaller pointing to lan78xx_write_reg routine.
Root-cause:
Proper cleanup of workqueues and init/setup routines was not happening
in failure conditions.
Fix:
Handled the error conditions by cleaning up the queues and init/setup
routines.
Fixes: 55d7de9de6c3 ("Microchip's LAN7800 family USB 2/3 to 10/100/1000 Ethernet device driver")
Reported-by: Andrey Konovalov <andreyknvl@google.com>
Signed-off-by: Raghuram Chary J <raghuramchary.jallipalli@microchip.com>
---
drivers/net/usb/lan78xx.c | 23 +++++++++++++++++++++--
1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index 60a604cc7647..11176070b345 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -2863,8 +2863,7 @@ static int lan78xx_bind(struct lan78xx_net *dev, struct usb_interface *intf)
if (ret < 0) {
netdev_warn(dev->net,
"lan78xx_setup_irq_domain() failed : %d", ret);
- kfree(pdata);
- return ret;
+ goto out1;
}
dev->net->hard_header_len += TX_OVERHEAD;
@@ -2872,14 +2871,32 @@ static int lan78xx_bind(struct lan78xx_net *dev, struct usb_interface *intf)
/* Init all registers */
ret = lan78xx_reset(dev);
+ if (ret) {
+ netdev_warn(dev->net, "Registers INIT FAILED....");
+ goto out2;
+ }
ret = lan78xx_mdio_init(dev);
+ if (ret) {
+ netdev_warn(dev->net, "MDIO INIT FAILED.....");
+ goto out2;
+ }
dev->net->flags |= IFF_MULTICAST;
pdata->wol = WAKE_MAGIC;
return ret;
+
+out2:
+ lan78xx_remove_irq_domain(dev);
+
+out1:
+ netdev_warn(dev->net, "Bind routine FAILED");
+ cancel_work_sync(&pdata->set_multicast);
+ cancel_work_sync(&pdata->set_vlan);
+ kfree(pdata);
+ return ret;
}
static void lan78xx_unbind(struct lan78xx_net *dev, struct usb_interface *intf)
@@ -2891,6 +2908,8 @@ static void lan78xx_unbind(struct lan78xx_net *dev, struct usb_interface *intf)
lan78xx_remove_mdio(dev);
if (pdata) {
+ cancel_work_sync(&pdata->set_multicast);
+ cancel_work_sync(&pdata->set_vlan);
netif_dbg(dev, ifdown, dev->net, "free pdata");
kfree(pdata);
pdata = NULL;
--
2.16.2
^ permalink raw reply related
* [PATCH iproute2-next] tc: Fix compilation error with old iptables
From: Roi Dayan @ 2018-03-27 9:20 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, Stephen Hemminger, Eyal Birger, Roi Dayan
The compat_rev field does not exists in old versions of iptables.
e.g. iptables 1.4.
Fixes: dd29621578d2 ("tc: add em_ipt ematch for calling xtables matches from tc matching context")
Signed-off-by: Roi Dayan <roid@mellanox.com>
---
tc/em_ipt.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tc/em_ipt.c b/tc/em_ipt.c
index 9d2b2f9b46d4..aa2edd63c550 100644
--- a/tc/em_ipt.c
+++ b/tc/em_ipt.c
@@ -41,7 +41,9 @@ static struct xtables_globals em_tc_ipt_globals = {
.program_version = "0.1",
.orig_opts = original_opts,
.opts = original_opts,
+#if (XTABLES_VERSION_CODE >= 11)
.compat_rev = xtables_compatible_revision,
+#endif
};
static struct xt_entry_match *fake_xt_entry_match(int data_size, void *data)
--
2.7.0
^ permalink raw reply related
* Re: [PATCH net] vhost: correctly remove wait queue during poll failure
From: Darren Kenny @ 2018-03-27 9:28 UTC (permalink / raw)
To: Jason Wang; +Cc: mst, kvm, virtualization, netdev, linux-kernel
In-Reply-To: <1522122442-30317-1-git-send-email-jasowang@redhat.com>
Hi Jason,
On Tue, Mar 27, 2018 at 11:47:22AM +0800, Jason Wang wrote:
>We tried to remove vq poll from wait queue, but do not check whether
>or not it was in a list before. This will lead double free. Fixing
>this by checking poll->wqh to make sure it was in a list.
This text seems at odds with the code below, instead of checking
poll-whq, you are removing that check...
Maybe the text needs rewording?
Thanks,
Darren.
>
>Reported-by: syzbot+c0272972b01b872e604a@syzkaller.appspotmail.com
>Fixes: 2b8b328b61c79 ("vhost_net: handle polling errors when setting backend")
>Signed-off-by: Jason Wang <jasowang@redhat.com>
>---
> drivers/vhost/vhost.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
>diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
>index 1b3e8d2d..5d5a9d9 100644
>--- a/drivers/vhost/vhost.c
>+++ b/drivers/vhost/vhost.c
>@@ -212,8 +212,7 @@ int vhost_poll_start(struct vhost_poll *poll, struct file *file)
> if (mask)
> vhost_poll_wakeup(&poll->wait, 0, 0, poll_to_key(mask));
> if (mask & EPOLLERR) {
>- if (poll->wqh)
>- remove_wait_queue(poll->wqh, &poll->wait);
>+ vhost_poll_stop(poll);
> ret = -EINVAL;
> }
>
>--
>2.7.4
>
^ permalink raw reply
* [bpf-next V6 PATCH 00/15] XDP redirect memory return API
From: Jesper Dangaard Brouer @ 2018-03-27 9:31 UTC (permalink / raw)
To: netdev, BjörnTöpel, magnus.karlsson
Cc: eugenia, Jason Wang, John Fastabend, Eran Ben Elisha,
Saeed Mahameed, galp, Jesper Dangaard Brouer, Daniel Borkmann,
Alexei Starovoitov, Tariq Toukan
This patchset works towards supporting different XDP RX-ring memory
allocators. As this will be needed by the AF_XDP zero-copy mode.
The patchset uses mlx5 as the sample driver, which gets implemented
XDP_REDIRECT RX-mode, but not ndo_xdp_xmit (as this API is subject to
change thought the patchset).
A new struct xdp_frame is introduced (modeled after cpumap xdp_pkt).
And both ndo_xdp_xmit and the new xdp_return_frame end-up using this.
Support for a driver supplied allocator is implemented, and a
refurbished version of page_pool is the first return allocator type
introduced. This will be a integration point for AF_XDP zero-copy.
The mlx5 driver evolve into using the page_pool, and see a performance
increase (with ndo_xdp_xmit out ixgbe driver) from 6Mpps to 12Mpps.
The patchset stop at the 15 patches limit, but more API changes are
planned. Specifically extending ndo_xdp_xmit and xdp_return_frame
APIs to support bulking. As this will address some known limits.
V2: Updated according to Tariq's feedback
V3: Updated based on feedback from Jason Wang and Alex Duyck
V4: Updated based on feedback from Tariq and Jason
V5: Fix SPDX license, add Tariq's reviews, improve patch desc for perf test
V6: Updated based on feedback from Eric Dumazet and Alex Duyck
---
Jesper Dangaard Brouer (15):
mlx5: basic XDP_REDIRECT forward support
xdp: introduce xdp_return_frame API and use in cpumap
ixgbe: use xdp_return_frame API
xdp: move struct xdp_buff from filter.h to xdp.h
xdp: introduce a new xdp_frame type
tun: convert to use generic xdp_frame and xdp_return_frame API
virtio_net: convert to use generic xdp_frame and xdp_return_frame API
bpf: cpumap convert to use generic xdp_frame
mlx5: register a memory model when XDP is enabled
xdp: rhashtable with allocator ID to pointer mapping
page_pool: refurbish version of page_pool code
xdp: allow page_pool as an allocator type in xdp_return_frame
mlx5: use page_pool for xdp_return_frame call
xdp: transition into using xdp_frame for return API
xdp: transition into using xdp_frame for ndo_xdp_xmit
drivers/net/ethernet/intel/ixgbe/ixgbe.h | 3
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 37 ++
drivers/net/ethernet/mellanox/mlx5/core/en.h | 4
drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 36 ++
drivers/net/ethernet/mellanox/mlx5/core/en_rx.c | 42 ++-
drivers/net/tun.c | 60 ++--
drivers/net/virtio_net.c | 52 ++-
drivers/vhost/net.c | 7
include/linux/filter.h | 24 --
include/linux/if_tun.h | 4
include/linux/netdevice.h | 4
include/net/page_pool.h | 129 +++++++++
include/net/xdp.h | 83 +++++
kernel/bpf/cpumap.c | 132 +++------
net/core/Makefile | 1
net/core/filter.c | 17 +
net/core/page_pool.c | 317 +++++++++++++++++++++
net/core/xdp.c | 258 +++++++++++++++++
18 files changed, 1034 insertions(+), 176 deletions(-)
create mode 100644 include/net/page_pool.h
create mode 100644 net/core/page_pool.c
^ permalink raw reply
* [bpf-next V6 PATCH 01/15] mlx5: basic XDP_REDIRECT forward support
From: Jesper Dangaard Brouer @ 2018-03-27 9:32 UTC (permalink / raw)
To: netdev, BjörnTöpel, magnus.karlsson
Cc: eugenia, Jason Wang, John Fastabend, Eran Ben Elisha,
Saeed Mahameed, galp, Jesper Dangaard Brouer, Daniel Borkmann,
Alexei Starovoitov, Tariq Toukan
In-Reply-To: <152214307922.9023.12528985070753859526.stgit@firesoul>
This implements basic XDP redirect support in mlx5 driver.
Notice that the ndo_xdp_xmit() is NOT implemented, because that API
need some changes that this patchset is working towards.
The main purpose of this patch is have different drivers doing
XDP_REDIRECT to show how different memory models behave in a cross
driver world.
Update(pre-RFCv2 Tariq): Need to DMA unmap page before xdp_do_redirect,
as the return API does not exist yet to to keep this mapped.
Update(pre-RFCv3 Saeed): Don't mix XDP_TX and XDP_REDIRECT flushing,
introduce xdpsq.db.redirect_flush boolian.
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
Reviewed-by: Tariq Toukan <tariqt@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx5/core/en.h | 1 +
drivers/net/ethernet/mellanox/mlx5/core/en_rx.c | 27 ++++++++++++++++++++---
2 files changed, 25 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h b/drivers/net/ethernet/mellanox/mlx5/core/en.h
index 4c9360b25532..28cc26debeda 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
@@ -398,6 +398,7 @@ struct mlx5e_xdpsq {
struct {
struct mlx5e_dma_info *di;
bool doorbell;
+ bool redirect_flush;
} db;
/* read only */
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
index 8cce90dc461d..6dcc3e8fbd3e 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
@@ -236,14 +236,20 @@ static inline int mlx5e_page_alloc_mapped(struct mlx5e_rq *rq,
return 0;
}
+static inline void mlx5e_page_dma_unmap(struct mlx5e_rq *rq,
+ struct mlx5e_dma_info *dma_info)
+{
+ dma_unmap_page(rq->pdev, dma_info->addr, RQ_PAGE_SIZE(rq),
+ rq->buff.map_dir);
+}
+
void mlx5e_page_release(struct mlx5e_rq *rq, struct mlx5e_dma_info *dma_info,
bool recycle)
{
if (likely(recycle) && mlx5e_rx_cache_put(rq, dma_info))
return;
- dma_unmap_page(rq->pdev, dma_info->addr, RQ_PAGE_SIZE(rq),
- rq->buff.map_dir);
+ mlx5e_page_dma_unmap(rq, dma_info);
put_page(dma_info->page);
}
@@ -822,9 +828,10 @@ static inline int mlx5e_xdp_handle(struct mlx5e_rq *rq,
struct mlx5e_dma_info *di,
void *va, u16 *rx_headroom, u32 *len)
{
- const struct bpf_prog *prog = READ_ONCE(rq->xdp_prog);
+ struct bpf_prog *prog = READ_ONCE(rq->xdp_prog);
struct xdp_buff xdp;
u32 act;
+ int err;
if (!prog)
return false;
@@ -845,6 +852,15 @@ static inline int mlx5e_xdp_handle(struct mlx5e_rq *rq,
if (unlikely(!mlx5e_xmit_xdp_frame(rq, di, &xdp)))
trace_xdp_exception(rq->netdev, prog, act);
return true;
+ case XDP_REDIRECT:
+ /* When XDP enabled then page-refcnt==1 here */
+ err = xdp_do_redirect(rq->netdev, &xdp, prog);
+ if (!err) {
+ rq->wqe.xdp_xmit = true; /* XDP xmit owns page */
+ rq->xdpsq.db.redirect_flush = true;
+ mlx5e_page_dma_unmap(rq, di);
+ }
+ return true;
default:
bpf_warn_invalid_xdp_action(act);
case XDP_ABORTED:
@@ -1107,6 +1123,11 @@ int mlx5e_poll_rx_cq(struct mlx5e_cq *cq, int budget)
xdpsq->db.doorbell = false;
}
+ if (xdpsq->db.redirect_flush) {
+ xdp_do_flush_map();
+ xdpsq->db.redirect_flush = false;
+ }
+
mlx5_cqwq_update_db_record(&cq->wq);
/* ensure cq space is freed before enabling more cqes */
^ permalink raw reply related
* [bpf-next V6 PATCH 02/15] xdp: introduce xdp_return_frame API and use in cpumap
From: Jesper Dangaard Brouer @ 2018-03-27 9:32 UTC (permalink / raw)
To: netdev, BjörnTöpel, magnus.karlsson
Cc: eugenia, Jason Wang, John Fastabend, Eran Ben Elisha,
Saeed Mahameed, galp, Jesper Dangaard Brouer, Daniel Borkmann,
Alexei Starovoitov, Tariq Toukan
In-Reply-To: <152214307922.9023.12528985070753859526.stgit@firesoul>
Introduce an xdp_return_frame API, and convert over cpumap as
the first user, given it have queued XDP frame structure to leverage.
V3: Cleanup and remove C99 style comments, pointed out by Alex Duyck.
V6: Remove comment that id will be added later (Req by Alex Duyck)
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
include/net/xdp.h | 27 +++++++++++++++++++++++
kernel/bpf/cpumap.c | 60 +++++++++++++++++++++++++++++++--------------------
net/core/xdp.c | 18 +++++++++++++++
3 files changed, 81 insertions(+), 24 deletions(-)
diff --git a/include/net/xdp.h b/include/net/xdp.h
index b2362ddfa694..257762e8a3ad 100644
--- a/include/net/xdp.h
+++ b/include/net/xdp.h
@@ -33,16 +33,43 @@
* also mandatory during RX-ring setup.
*/
+enum mem_type {
+ MEM_TYPE_PAGE_SHARED = 0, /* Split-page refcnt based model */
+ MEM_TYPE_PAGE_ORDER0, /* Orig XDP full page model */
+ MEM_TYPE_MAX,
+};
+
+struct xdp_mem_info {
+ u32 type; /* enum mem_type, but known size type */
+};
+
struct xdp_rxq_info {
struct net_device *dev;
u32 queue_index;
u32 reg_state;
+ struct xdp_mem_info mem;
} ____cacheline_aligned; /* perf critical, avoid false-sharing */
+
+static inline
+void xdp_return_frame(void *data, struct xdp_mem_info *mem)
+{
+ if (mem->type == MEM_TYPE_PAGE_SHARED)
+ page_frag_free(data);
+
+ if (mem->type == MEM_TYPE_PAGE_ORDER0) {
+ struct page *page = virt_to_page(data); /* Assumes order0 page*/
+
+ put_page(page);
+ }
+}
+
int xdp_rxq_info_reg(struct xdp_rxq_info *xdp_rxq,
struct net_device *dev, u32 queue_index);
void xdp_rxq_info_unreg(struct xdp_rxq_info *xdp_rxq);
void xdp_rxq_info_unused(struct xdp_rxq_info *xdp_rxq);
bool xdp_rxq_info_is_reg(struct xdp_rxq_info *xdp_rxq);
+int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
+ enum mem_type type, void *allocator);
#endif /* __LINUX_NET_XDP_H__ */
diff --git a/kernel/bpf/cpumap.c b/kernel/bpf/cpumap.c
index a4bb0b34375a..3e4bbcbe3e86 100644
--- a/kernel/bpf/cpumap.c
+++ b/kernel/bpf/cpumap.c
@@ -19,6 +19,7 @@
#include <linux/bpf.h>
#include <linux/filter.h>
#include <linux/ptr_ring.h>
+#include <net/xdp.h>
#include <linux/sched.h>
#include <linux/workqueue.h>
@@ -137,27 +138,6 @@ static struct bpf_map *cpu_map_alloc(union bpf_attr *attr)
return ERR_PTR(err);
}
-static void __cpu_map_queue_destructor(void *ptr)
-{
- /* The tear-down procedure should have made sure that queue is
- * empty. See __cpu_map_entry_replace() and work-queue
- * invoked cpu_map_kthread_stop(). Catch any broken behaviour
- * gracefully and warn once.
- */
- if (WARN_ON_ONCE(ptr))
- page_frag_free(ptr);
-}
-
-static void put_cpu_map_entry(struct bpf_cpu_map_entry *rcpu)
-{
- if (atomic_dec_and_test(&rcpu->refcnt)) {
- /* The queue should be empty at this point */
- ptr_ring_cleanup(rcpu->queue, __cpu_map_queue_destructor);
- kfree(rcpu->queue);
- kfree(rcpu);
- }
-}
-
static void get_cpu_map_entry(struct bpf_cpu_map_entry *rcpu)
{
atomic_inc(&rcpu->refcnt);
@@ -188,6 +168,10 @@ struct xdp_pkt {
u16 len;
u16 headroom;
u16 metasize;
+ /* Lifetime of xdp_rxq_info is limited to NAPI/enqueue time,
+ * while mem info is valid on remote CPU.
+ */
+ struct xdp_mem_info mem;
struct net_device *dev_rx;
};
@@ -213,6 +197,9 @@ static struct xdp_pkt *convert_to_xdp_pkt(struct xdp_buff *xdp)
xdp_pkt->headroom = headroom - sizeof(*xdp_pkt);
xdp_pkt->metasize = metasize;
+ /* rxq only valid until napi_schedule ends, convert to xdp_mem_info */
+ xdp_pkt->mem = xdp->rxq->mem;
+
return xdp_pkt;
}
@@ -265,6 +252,31 @@ static struct sk_buff *cpu_map_build_skb(struct bpf_cpu_map_entry *rcpu,
return skb;
}
+static void __cpu_map_ring_cleanup(struct ptr_ring *ring)
+{
+ /* The tear-down procedure should have made sure that queue is
+ * empty. See __cpu_map_entry_replace() and work-queue
+ * invoked cpu_map_kthread_stop(). Catch any broken behaviour
+ * gracefully and warn once.
+ */
+ struct xdp_pkt *xdp_pkt;
+
+ while ((xdp_pkt = ptr_ring_consume(ring)))
+ if (WARN_ON_ONCE(xdp_pkt))
+ xdp_return_frame(xdp_pkt, &xdp_pkt->mem);
+}
+
+static void put_cpu_map_entry(struct bpf_cpu_map_entry *rcpu)
+{
+ if (atomic_dec_and_test(&rcpu->refcnt)) {
+ /* The queue should be empty at this point */
+ __cpu_map_ring_cleanup(rcpu->queue);
+ ptr_ring_cleanup(rcpu->queue, NULL);
+ kfree(rcpu->queue);
+ kfree(rcpu);
+ }
+}
+
static int cpu_map_kthread_run(void *data)
{
struct bpf_cpu_map_entry *rcpu = data;
@@ -307,7 +319,7 @@ static int cpu_map_kthread_run(void *data)
skb = cpu_map_build_skb(rcpu, xdp_pkt);
if (!skb) {
- page_frag_free(xdp_pkt);
+ xdp_return_frame(xdp_pkt, &xdp_pkt->mem);
continue;
}
@@ -604,13 +616,13 @@ static int bq_flush_to_queue(struct bpf_cpu_map_entry *rcpu,
spin_lock(&q->producer_lock);
for (i = 0; i < bq->count; i++) {
- void *xdp_pkt = bq->q[i];
+ struct xdp_pkt *xdp_pkt = bq->q[i];
int err;
err = __ptr_ring_produce(q, xdp_pkt);
if (err) {
drops++;
- page_frag_free(xdp_pkt); /* Free xdp_pkt */
+ xdp_return_frame(xdp_pkt->data, &xdp_pkt->mem);
}
processed++;
}
diff --git a/net/core/xdp.c b/net/core/xdp.c
index 097a0f74e004..9eee0c431126 100644
--- a/net/core/xdp.c
+++ b/net/core/xdp.c
@@ -71,3 +71,21 @@ bool xdp_rxq_info_is_reg(struct xdp_rxq_info *xdp_rxq)
return (xdp_rxq->reg_state == REG_STATE_REGISTERED);
}
EXPORT_SYMBOL_GPL(xdp_rxq_info_is_reg);
+
+int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
+ enum mem_type type, void *allocator)
+{
+ if (type >= MEM_TYPE_MAX)
+ return -EINVAL;
+
+ xdp_rxq->mem.type = type;
+
+ if (allocator)
+ return -EOPNOTSUPP;
+
+ /* TODO: Allocate an ID that maps to allocator pointer
+ * See: https://www.kernel.org/doc/html/latest/core-api/idr.html
+ */
+ return 0;
+}
+EXPORT_SYMBOL_GPL(xdp_rxq_info_reg_mem_model);
^ permalink raw reply related
* [bpf-next V6 PATCH 03/15] ixgbe: use xdp_return_frame API
From: Jesper Dangaard Brouer @ 2018-03-27 9:32 UTC (permalink / raw)
To: netdev, BjörnTöpel, magnus.karlsson
Cc: eugenia, Jason Wang, John Fastabend, Eran Ben Elisha,
Saeed Mahameed, galp, Jesper Dangaard Brouer, Daniel Borkmann,
Alexei Starovoitov, Tariq Toukan
In-Reply-To: <152214307922.9023.12528985070753859526.stgit@firesoul>
Extend struct ixgbe_tx_buffer to store the xdp_mem_info.
Notice that this could be optimized further by putting this into
a union in the struct ixgbe_tx_buffer, but this patchset
works towards removing this again. Thus, this is not done.
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe.h | 1 +
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 6 ++++--
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index c1e3a0039ea5..cbc20f199364 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -249,6 +249,7 @@ struct ixgbe_tx_buffer {
DEFINE_DMA_UNMAP_ADDR(dma);
DEFINE_DMA_UNMAP_LEN(len);
u32 tx_flags;
+ struct xdp_mem_info xdp_mem;
};
struct ixgbe_rx_buffer {
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 85369423452d..45520eb503ee 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -1207,7 +1207,7 @@ static bool ixgbe_clean_tx_irq(struct ixgbe_q_vector *q_vector,
/* free the skb */
if (ring_is_xdp(tx_ring))
- page_frag_free(tx_buffer->data);
+ xdp_return_frame(tx_buffer->data, &tx_buffer->xdp_mem);
else
napi_consume_skb(tx_buffer->skb, napi_budget);
@@ -5787,7 +5787,7 @@ static void ixgbe_clean_tx_ring(struct ixgbe_ring *tx_ring)
/* Free all the Tx ring sk_buffs */
if (ring_is_xdp(tx_ring))
- page_frag_free(tx_buffer->data);
+ xdp_return_frame(tx_buffer->data, &tx_buffer->xdp_mem);
else
dev_kfree_skb_any(tx_buffer->skb);
@@ -8351,6 +8351,8 @@ static int ixgbe_xmit_xdp_ring(struct ixgbe_adapter *adapter,
dma_unmap_len_set(tx_buffer, len, len);
dma_unmap_addr_set(tx_buffer, dma, dma);
tx_buffer->data = xdp->data;
+ tx_buffer->xdp_mem = xdp->rxq->mem;
+
tx_desc->read.buffer_addr = cpu_to_le64(dma);
/* put descriptor type bits */
^ permalink raw reply related
* [bpf-next V6 PATCH 04/15] xdp: move struct xdp_buff from filter.h to xdp.h
From: Jesper Dangaard Brouer @ 2018-03-27 9:32 UTC (permalink / raw)
To: netdev, BjörnTöpel, magnus.karlsson
Cc: eugenia, Jason Wang, John Fastabend, Eran Ben Elisha,
Saeed Mahameed, galp, Jesper Dangaard Brouer, Daniel Borkmann,
Alexei Starovoitov, Tariq Toukan
In-Reply-To: <152214307922.9023.12528985070753859526.stgit@firesoul>
This is done to prepare for the next patch, and it is also
nice to move this XDP related struct out of filter.h.
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
include/linux/filter.h | 24 +-----------------------
include/net/xdp.h | 22 ++++++++++++++++++++++
2 files changed, 23 insertions(+), 23 deletions(-)
diff --git a/include/linux/filter.h b/include/linux/filter.h
index 109d05ccea9a..340ba653dd80 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -30,6 +30,7 @@ struct sock;
struct seccomp_data;
struct bpf_prog_aux;
struct xdp_rxq_info;
+struct xdp_buff;
/* ArgX, context and stack frame pointer register positions. Note,
* Arg1, Arg2, Arg3, etc are used as argument mappings of function
@@ -499,14 +500,6 @@ struct bpf_skb_data_end {
void *data_end;
};
-struct xdp_buff {
- void *data;
- void *data_end;
- void *data_meta;
- void *data_hard_start;
- struct xdp_rxq_info *rxq;
-};
-
struct sk_msg_buff {
void *data;
void *data_end;
@@ -769,21 +762,6 @@ int xdp_do_redirect(struct net_device *dev,
struct bpf_prog *prog);
void xdp_do_flush_map(void);
-/* Drivers not supporting XDP metadata can use this helper, which
- * rejects any room expansion for metadata as a result.
- */
-static __always_inline void
-xdp_set_data_meta_invalid(struct xdp_buff *xdp)
-{
- xdp->data_meta = xdp->data + 1;
-}
-
-static __always_inline bool
-xdp_data_meta_unsupported(const struct xdp_buff *xdp)
-{
- return unlikely(xdp->data_meta > xdp->data);
-}
-
void bpf_warn_invalid_xdp_action(u32 act);
struct sock *do_sk_redirect_map(struct sk_buff *skb);
diff --git a/include/net/xdp.h b/include/net/xdp.h
index 257762e8a3ad..48cc5d3ee10e 100644
--- a/include/net/xdp.h
+++ b/include/net/xdp.h
@@ -50,6 +50,13 @@ struct xdp_rxq_info {
struct xdp_mem_info mem;
} ____cacheline_aligned; /* perf critical, avoid false-sharing */
+struct xdp_buff {
+ void *data;
+ void *data_end;
+ void *data_meta;
+ void *data_hard_start;
+ struct xdp_rxq_info *rxq;
+};
static inline
void xdp_return_frame(void *data, struct xdp_mem_info *mem)
@@ -72,4 +79,19 @@ bool xdp_rxq_info_is_reg(struct xdp_rxq_info *xdp_rxq);
int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
enum mem_type type, void *allocator);
+/* Drivers not supporting XDP metadata can use this helper, which
+ * rejects any room expansion for metadata as a result.
+ */
+static __always_inline void
+xdp_set_data_meta_invalid(struct xdp_buff *xdp)
+{
+ xdp->data_meta = xdp->data + 1;
+}
+
+static __always_inline bool
+xdp_data_meta_unsupported(const struct xdp_buff *xdp)
+{
+ return unlikely(xdp->data_meta > xdp->data);
+}
+
#endif /* __LINUX_NET_XDP_H__ */
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox