* [net-next:master 518/519] drivers/net/virtio_net.c:1910:3: error: implicit declaration of function '__netif_set_xps_queue'; did you mean 'netif_set_xps_queue'?
From: kbuild test robot @ 2018-08-09 23:34 UTC (permalink / raw)
To: Andrei Vagin; +Cc: kbuild-all, netdev
[-- Attachment #1: Type: text/plain, Size: 2204 bytes --]
tree: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git master
head: 36d2f761b5aa688567b6aebdc6d68e73682275d4
commit: 4d99f6602cb552fb58db0c3b1d935bb6fa017f24 [518/519] net: allow to call netif_reset_xps_queues() under cpus_read_lock
config: sh-allmodconfig (attached as .config)
compiler: sh4-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
git checkout 4d99f6602cb552fb58db0c3b1d935bb6fa017f24
# save the attached .config to linux build tree
GCC_VERSION=7.2.0 make.cross ARCH=sh
All errors (new ones prefixed by >>):
drivers/net/virtio_net.c: In function 'virtnet_set_affinity':
>> drivers/net/virtio_net.c:1910:3: error: implicit declaration of function '__netif_set_xps_queue'; did you mean 'netif_set_xps_queue'? [-Werror=implicit-function-declaration]
__netif_set_xps_queue(vi->dev, mask, i, false);
^~~~~~~~~~~~~~~~~~~~~
netif_set_xps_queue
cc1: some warnings being treated as errors
vim +1910 drivers/net/virtio_net.c
1888
1889 static void virtnet_set_affinity(struct virtnet_info *vi)
1890 {
1891 int i;
1892 int cpu;
1893
1894 /* In multiqueue mode, when the number of cpu is equal to the number of
1895 * queue pairs, we let the queue pairs to be private to one cpu by
1896 * setting the affinity hint to eliminate the contention.
1897 */
1898 if (vi->curr_queue_pairs == 1 ||
1899 vi->max_queue_pairs != num_online_cpus()) {
1900 virtnet_clean_affinity(vi, -1);
1901 return;
1902 }
1903
1904 i = 0;
1905 for_each_online_cpu(cpu) {
1906 const unsigned long *mask = cpumask_bits(cpumask_of(cpu));
1907
1908 virtqueue_set_affinity(vi->rq[i].vq, cpu);
1909 virtqueue_set_affinity(vi->sq[i].vq, cpu);
> 1910 __netif_set_xps_queue(vi->dev, mask, i, false);
1911 i++;
1912 }
1913
1914 vi->affinity_hint_set = true;
1915 }
1916
---
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: 48753 bytes --]
^ permalink raw reply
* pull-request: bpf 2018-08-10
From: Daniel Borkmann @ 2018-08-09 23:21 UTC (permalink / raw)
To: davem; +Cc: daniel, ast, netdev
Hi David,
The following pull-request contains BPF updates for your *net* tree.
The main changes are:
1) Fix cpumap and devmap on teardown as they're under RCU context
and won't have same assumption as running under NAPI protection,
from Jesper.
2) Fix various sockmap bugs in bpf_tcp_sendmsg() code, e.g. we had
a bug where socket error was not propagated correctly, from Daniel.
3) Fix incompatible libbpf header license for BTF code and match it
before it gets officially released with the rest of libbpf which
is LGPL-2.1, from Martin.
Please consider pulling these changes from:
git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git
Thanks a lot!
----------------------------------------------------------------
The following changes since commit 82a40777de12728dedf4075453b694f0d1baee80:
ip6_tunnel: use the right value for ipv4 min mtu check in ip6_tnl_xmit (2018-08-05 17:35:02 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git
for you to fetch changes up to 9c95420117393ed5f76de373e3c6479c21e3e380:
Merge branch 'bpf-fix-cpu-and-devmap-teardown' (2018-08-09 21:50:45 +0200)
----------------------------------------------------------------
Alexei Starovoitov (1):
Merge branch 'sockmap-fixes'
Daniel Borkmann (4):
bpf, sockmap: fix bpf_tcp_sendmsg sock error handling
bpf, sockmap: fix leak in bpf_tcp_sendmsg wait for mem path
bpf, sockmap: fix cork timeout for select due to epipe
Merge branch 'bpf-fix-cpu-and-devmap-teardown'
Jesper Dangaard Brouer (3):
xdp: fix bug in cpumap teardown code path
samples/bpf: xdp_redirect_cpu adjustment to reproduce teardown race easier
xdp: fix bug in devmap teardown code path
Martin KaFai Lau (1):
bpf: btf: Change tools/lib/bpf/btf to LGPL
kernel/bpf/cpumap.c | 15 +++++++++------
kernel/bpf/devmap.c | 14 +++++++++-----
kernel/bpf/sockmap.c | 9 ++++++---
samples/bpf/xdp_redirect_cpu_kern.c | 2 +-
samples/bpf/xdp_redirect_cpu_user.c | 4 ++--
tools/lib/bpf/btf.c | 2 +-
tools/lib/bpf/btf.h | 2 +-
tools/testing/selftests/bpf/test_sockmap.c | 2 +-
8 files changed, 30 insertions(+), 20 deletions(-)
^ permalink raw reply
* Re: [PATCH bpf-next 0/4] Convert filter.txt to RST
From: Tobin C. Harding @ 2018-08-10 1:46 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Alexei Starovoitov, Alexei Starovoitov, Jonathan Corbet,
David S. Miller, Kees Cook, Andy Lutomirski, Will Drewry,
linux-doc, netdev, linux-kernel
In-Reply-To: <80bdc251-9f57-602f-6536-b34651684bb7@iogearbox.net>
On Thu, Aug 09, 2018 at 10:24:54AM +0200, Daniel Borkmann wrote:
> On 08/09/2018 09:27 AM, Tobin C. Harding wrote:
> > On Wed, Aug 08, 2018 at 11:07:35PM -0700, Alexei Starovoitov wrote:
> >> On Thu, Aug 09, 2018 at 03:23:24PM +1000, Tobin C. Harding wrote:
> >>>
> >>> Daniel and Alexei, can I please have permission to add GPLv2+ to the BPF
> >>> docs?
> >>
> >> kernel licensing is GPLv2 without +
> >
> > According to process/license-rules.rst
> >
> > GPL-2.0+ : GNU General Public License v2.0 or later
>
> Not really, please see the first three paragraphs of process/license-rules.rst.
> The COPYING file of the kernel says that it's 'v2' and not 'v2 or later',
> unless otherwise _explicitly_ noted. Given that and given there is no other
> specific note in filter.txt, it would mean it's v2-only due to that rule.
Thanks for clarifying. My understanding is now; this is a case where
checkpatch is too verbose and we do not actually need to add a specific
license identifier to the documentation files (new or otherwise). They
get an implicit GPLv2.
I'll remove the licences identifiers and re-spin.
thanks,
Tobin.
^ permalink raw reply
* [net-next:master 1939/1953] drivers/net/ethernet/qlogic/qede/qede_main.c:539:5: sparse: symbol 'qede_setup_tc' was not declared. Should it be static?
From: kbuild test robot @ 2018-08-10 1:29 UTC (permalink / raw)
To: Manish Chopra
Cc: kbuild-all, netdev, Ariel Elior, everest-linux-l2, linux-kernel
tree: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git master
head: 36d2f761b5aa688567b6aebdc6d68e73682275d4
commit: 5e7baf0fcb2a3aef7329f3c7543d4695a46bd321 [1939/1953] qed/qede: Multi CoS support.
reproduce:
# apt-get install sparse
git checkout 5e7baf0fcb2a3aef7329f3c7543d4695a46bd321
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__
sparse warnings: (new ones prefixed by >>)
>> drivers/net/ethernet/qlogic/qede/qede_main.c:539:5: sparse: symbol 'qede_setup_tc' was not declared. Should it be static?
include/linux/slab.h:631:13: sparse: undefined identifier '__builtin_mul_overflow'
include/linux/slab.h:631:13: sparse: not a function <noident>
drivers/net/ethernet/qlogic/qede/qede_main.c:1175:19: sparse: expression using sizeof(void)
drivers/net/ethernet/qlogic/qede/qede_main.c:1175:19: sparse: expression using sizeof(void)
include/linux/slab.h:631:13: sparse: call with no type!
Please review and possibly fold the followup patch.
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply
* Re: [PATCH] net: ethernet: cpsw-phy-sel: prefer phandle for phy sel and update binding
From: Grygorii Strashko @ 2018-08-09 22:47 UTC (permalink / raw)
To: Tony Lindgren, Andrew Lunn
Cc: David Miller, netdev, linux-omap, devicetree, Ivan Khoronzhuk,
Mark Rutland, Murali Karicheri, Rob Herring
In-Reply-To: <20180809104653.GV99251@atomide.com>
On 08/09/2018 05:46 AM, Tony Lindgren wrote:
> * Tony Lindgren <tony@atomide.com> [180808 13:52]:
>> * Andrew Lunn <andrew@lunn.ch> [180808 12:02]:
>>>
>>> Do you need to handle EPROBE_DEFER here? The phandle points to a
>>> device which has not yet been loaded? I'm not sure exactly where it
>>> will be returned, maybe it is bus_find_device(), but i expect to see
>>> some handling of it somewhere in this function.
>
> If no device is found the driver just produces a warning currently.
> And in that case cpsw attempts to continue with bootloader settings.
>
> And looking at the caller function cpsw_slave_open() it also just
> produces warnings for phy_connect() too..
>
> I agree that in general this this whole pile of cpsw related drivers sure
> could use some better error handling. Starting with making cpsw_slave_open()
> and cpsw_phy_sel() return errors instead of just ignoring them might be a
> good start.
>
> Grygorii, care to add that note of things to do into your cpsw maintainer
> hat?
Right. EPROBE_DEFER not supported for this module as of now.
>
>> With the proper interconnect hierarchy in the device tree there should be
>> no EPROBE_DEFER happening here as the interconnects are probed in the
>> right order with the always on interrupt with system control module first :)
>>
>> But then again, adding support for EPROBE_DEFER here won't hurt either,
>> will take a look.
>
> I'll just add some notes about that to the patch description considering
> the above.
thanks Tony.
--
regards,
-grygorii
^ permalink raw reply
* Re: [PATCH lora-next v2 8/8] net: lora: sx1301: convert driver over to regmap reads and writes
From: Ben Whitten @ 2018-08-09 22:47 UTC (permalink / raw)
To: Andreas Färber
Cc: 潘建宏, hasnain.virk, netdev, Xue Liu, shess,
Ben Whitten, yannick.lanz
In-Reply-To: <e7b112d9-d8ac-1654-fc97-9289180d6a02@suse.de>
On Thu, 9 Aug 2018 at 23:34, Andreas Färber <afaerber@suse.de> wrote:
>
> Am 09.08.2018 um 14:33 schrieb Ben Whitten:
> > The reads and writes are replaced with regmap versions and unneeded
> > functions, variable, and defines removed.
> >
> > Signed-off-by: Ben Whitten <ben.whitten@lairdtech.com>
> > ---
> > drivers/net/lora/sx1301.c | 204 +++++++++++++++-------------------------------
> > drivers/net/lora/sx1301.h | 30 +++++++
> > 2 files changed, 95 insertions(+), 139 deletions(-)
> >
> > diff --git a/drivers/net/lora/sx1301.c b/drivers/net/lora/sx1301.c
> > index 766df06..4db5a43 100644
> > --- a/drivers/net/lora/sx1301.c
> > +++ b/drivers/net/lora/sx1301.c
> [...]
> > @@ -140,50 +115,9 @@ static int sx1301_write(struct sx1301_priv *priv, u8 reg, u8 val)
> > return sx1301_write_burst(priv, reg, &val, 1);
> > }
>
> _write and _read are now unused, causing warnings. Dropping.
>
> The _burst versions are still in use for firmware load, and I saw a
> discussion indicating that regmap is lacking the capability to not
> increment the reg for bulk reads at the moment. So we still can't
> cleanly switch to regmap entirely and thereby remain bound to SPI.
>
> [...]
> > @@ -235,8 +169,8 @@ static int sx1301_radio_spi_transfer_one(struct spi_controller *ctrl,
> > {
> > struct spi_sx1301 *ssx = spi_controller_get_devdata(ctrl);
> > struct sx1301_priv *priv = spi_get_drvdata(ssx->parent);
> > - const u8 *tx_buf = xfr->tx_buf;
> > - u8 *rx_buf = xfr->rx_buf;
> > + const unsigned int *tx_buf = xfr->tx_buf;
> > + unsigned int *rx_buf = xfr->rx_buf;
>
> These are wrong both for Little Endian and even worse for Big Endian.
>
> > int ret;
> >
> > if (xfr->len == 0 || xfr->len > 3)
> > @@ -245,13 +179,13 @@ static int sx1301_radio_spi_transfer_one(struct spi_controller *ctrl,
> > dev_dbg(&spi->dev, "transferring one (%u)\n", xfr->len);
> >
> > if (tx_buf) {
> > - ret = sx1301_page_write(priv, ssx->page, ssx->regs + REG_RADIO_X_ADDR, tx_buf ? tx_buf[0] : 0);
> > + ret = regmap_write(priv->regmap, ssx->regs + REG_RADIO_X_ADDR, tx_buf ? tx_buf[0] : 0);
> > if (ret) {
> > dev_err(&spi->dev, "SPI radio address write failed\n");
> > return ret;
> > }
> >
> > - ret = sx1301_page_write(priv, ssx->page, ssx->regs + REG_RADIO_X_DATA, (tx_buf && xfr->len >= 2) ? tx_buf[1] : 0);
> > + ret = regmap_write(priv->regmap, ssx->regs + REG_RADIO_X_DATA, (tx_buf && xfr->len >= 2) ? tx_buf[1] : 0);
> > if (ret) {
> > dev_err(&spi->dev, "SPI radio data write failed\n");
> > return ret;
> > @@ -271,7 +205,7 @@ static int sx1301_radio_spi_transfer_one(struct spi_controller *ctrl,
> > }
> >
> > if (rx_buf) {
> > - ret = sx1301_page_read(priv, ssx->page, ssx->regs + REG_RADIO_X_DATA_READBACK, &rx_buf[xfr->len - 1]);
> > + ret = regmap_read(priv->regmap, ssx->regs + REG_RADIO_X_DATA_READBACK, &rx_buf[xfr->len - 1]);
> > if (ret) {
> > dev_err(&spi->dev, "SPI radio data read failed\n");
> > return ret;
>
> Fixing by adding a local variable instead:
>
> @@ -239,6 +163,7 @@ static int sx1301_radio_spi_transfer_one(struct
> spi_controll
> er *ctrl,
> struct sx1301_priv *priv = netdev_priv(netdev);
> const u8 *tx_buf = xfr->tx_buf;
> u8 *rx_buf = xfr->rx_buf;
> + unsigned int val;
> int ret;
>
> if (xfr->len == 0 || xfr->len > 3)
> [...]
> @@ -273,27 +198,28 @@ static int sx1301_radio_spi_transfer_one(struct
> spi_controller *ctrl,
> }
>
> if (rx_buf) {
> - ret = sx1301_page_read(priv, ssx->page, ssx->regs +
> REG_RADIO_X_DATA_READBACK, &rx_buf[xfr->len - 1]);
> + ret = regmap_read(priv->regmap, ssx->regs +
> REG_RADIO_X_DATA_READBACK, &val);
> if (ret) {
> dev_err(&spi->dev, "SPI radio data read failed\n");
> return ret;
> }
> + rx_buf[xfr->len - 1] = val & 0xff;
> }
>
> return 0;
>
> [...]
> > diff --git a/drivers/net/lora/sx1301.h b/drivers/net/lora/sx1301.h
> > index 2fc283f..b21e5c6 100644
> > --- a/drivers/net/lora/sx1301.h
> > +++ b/drivers/net/lora/sx1301.h
> > @@ -18,11 +18,41 @@
> > /* Page independent */
> > #define SX1301_PAGE 0x00
> > #define SX1301_VER 0x01
> > +#define SX1301_MPA 0x09
>
> Those are the official register names? I find these much harder to read
> than my guessed names. Could we keep the long names as aliases?
Yes these are the official register names, aliases to improve readability
sound like a good plan as all the official names are terse.
> > +#define SX1301_MPD 0x0A
> > +#define SX1301_GEN 0x10
> > +#define SX1301_CKEN 0x11
> > +#define SX1301_GPSO 0x1C
> > +#define SX1301_GPMODE 0x1D
> > +#define SX1301_AGCSTS 0x20
> >
> > #define SX1301_VIRT_BASE 0x100
> > #define SX1301_PAGE_LEN 0x80
> > #define SX1301_PAGE_BASE(n) (SX1301_VIRT_BASE + (SX1301_PAGE_LEN * n))
> >
> > +/* Page 0 */
> > +#define SX1301_CHRS (SX1301_PAGE_BASE(0) + 0x23)
> > +#define SX1301_FORCE_CTRL (SX1301_PAGE_BASE(0) + 0x69)
> > +#define SX1301_MCU_CTRL (SX1301_PAGE_BASE(0) + 0x6A)
> > +
> > +/* Page 2 */
> > +#define SX1301_RADIO_A_SPI_DATA (SX1301_PAGE_BASE(2) + 0x21)
> > +#define SX1301_RADIO_A_SPI_DATA_RB (SX1301_PAGE_BASE(2) + 0x22)
> > +#define SX1301_RADIO_A_SPI_ADDR (SX1301_PAGE_BASE(2) + 0x23)
> > +#define SX1301_RADIO_A_SPI_CS (SX1301_PAGE_BASE(2) + 0x25)
> > +#define SX1301_RADIO_B_SPI_DATA (SX1301_PAGE_BASE(2) + 0x26)
> > +#define SX1301_RADIO_B_SPI_DATA_RB (SX1301_PAGE_BASE(2) + 0x27)
> > +#define SX1301_RADIO_B_SPI_ADDR (SX1301_PAGE_BASE(2) + 0x28)
> > +#define SX1301_RADIO_B_SPI_CS (SX1301_PAGE_BASE(2) + 0x2A)
> > +#define SX1301_RADIO_CFG (SX1301_PAGE_BASE(2) + 0x2B)
> > +#define SX1301_DBG_ARB_MCU_RAM_DATA (SX1301_PAGE_BASE(2) + 0x40)
> > +#define SX1301_DBG_AGC_MCU_RAM_DATA (SX1301_PAGE_BASE(2) + 0x41)
> > +#define SX1301_DBG_ARB_MCU_RAM_ADDR (SX1301_PAGE_BASE(2) + 0x50)
> > +#define SX1301_DBG_AGC_MCU_RAM_ADDR (SX1301_PAGE_BASE(2) + 0x51)
> > +
> > +/* Page 3 */
> > +#define SX1301_EMERGENCY_FORCE_HOST_CTRL (SX1301_PAGE_BASE(3) + 0x7F)
> > +
> > #define SX1301_MAX_REGISTER (SX1301_PAGE_BASE(3) + 0x7F)
> >
> > #endif
>
> Applying so that we can continue based on regmap.
Thanks!
Ben Whitten
^ permalink raw reply
* Re: [PATCH net-next,v4] net/tls: Calculate nsg for zerocopy path without skb_cow_data.
From: Doron Roberts-Kedes @ 2018-08-09 22:43 UTC (permalink / raw)
To: David Miller; +Cc: davejwatson, vakul.garg, borisp, aviadye, netdev
In-Reply-To: <20180808.121430.2168057232319674612.davem@davemloft.net>
On Wed, Aug 08, 2018 at 12:14:30PM -0700, David Miller wrote:
> From: Doron Roberts-Kedes <doronrk@fb.com>
> Date: Tue, 7 Aug 2018 11:09:39 -0700
>
> > +static int __skb_nsg(struct sk_buff *skb, int offset, int len,
> > + unsigned int recursion_level)
> > +{
> > + int start = skb_headlen(skb);
> > + int i, copy = start - offset;
> > + struct sk_buff *frag_iter;
> > + int elt = 0;
> > +
> > + if (unlikely(recursion_level >= 24))
> > + return -EMSGSIZE;
>
> This recursion is kinda crazy.
>
> Even skb_cow_data() doesn't recurse like this (of course because it copies
> into linear buffers).
>
> There has to be a way to simplify this. Fragment lists are such a rarely
> used SKB geometry, and few if any devices support it for transmission
> (so the fraglist will get undone at transmit time anyways).
>
Interesting. Just wanted to clarify whether the issue is the use of
recursion or the fact that the function is handling the frag_list at
all. This is the rx path, so my understanding was that we need to handle
the frag_list. Please let me know if I'm misunderstanding your point
about the rare use of fragment lists.
If the issue is the recursion, I can rewrite the function to not use
recursion, but skb_to_sgvec uses a similar pattern and is invoked
immediately afterwards.
Taking a step back, is there an existing solution for what this function
is trying to do? I was surprised to find that there did not seem to
exist a function for determining the number of scatterlist elements
required to map an skb without COW.
^ permalink raw reply
* Re: [PATCH bpf-next] bpf: enable btf for use in all maps
From: Daniel Borkmann @ 2018-08-09 22:43 UTC (permalink / raw)
To: Alexei Starovoitov; +Cc: ast, netdev, yhs
In-Reply-To: <20180809214430.gd4zwsnmbwmq7b26@ast-mbp>
On 08/09/2018 11:44 PM, Alexei Starovoitov wrote:
> On Thu, Aug 09, 2018 at 11:30:52PM +0200, Daniel Borkmann wrote:
>> On 08/09/2018 11:14 PM, Alexei Starovoitov wrote:
>>> On Thu, Aug 09, 2018 at 09:42:20PM +0200, Daniel Borkmann wrote:
>>>> Commit a26ca7c982cb ("bpf: btf: Add pretty print support to
>>>> the basic arraymap") enabled support for BTF and dumping via
>>>> BPF fs for arraymap. However, both can be decoupled from each
>>>> other such that all BPF maps can be supported for attaching
>>>> BTF key/value information, while not all maps necessarily
>>>> need to dump via map_seq_show_elem() callback.
>>>>
>>>> The check in array_map_check_btf() can be generalized as
>>>> ultimatively the key and value size is the only contraint
>>>> that needs to match for the map. The fact that the key needs
>>>> to be of type int is optional; it could be any data type as
>>>> long as it matches the 4 byte key size, just like hash table
>>>> key or others could be of any data type as well.
>>>>
>>>> Minimal example of a hash table dump which then works out
>>>> of the box for bpftool:
>>>>
>>>> # bpftool map dump id 19
>>>> [{
>>>> "key": {
>>>> "": {
>>>> "vip": 0,
>>>> "vipv6": []
>>>> },
>>>> "port": 0,
>>>> "family": 0,
>>>> "proto": 0
>>>> },
>>>> "value": {
>>>> "flags": 0,
>>>> "vip_num": 0
>>>> }
>>>> }
>>>> ]
>>>>
>>>> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
>>>> Cc: Yonghong Song <yhs@fb.com>
>>>> ---
>>>> include/linux/bpf.h | 4 +---
>>>> kernel/bpf/arraymap.c | 27 ---------------------------
>>>> kernel/bpf/inode.c | 3 ++-
>>>> kernel/bpf/syscall.c | 24 ++++++++++++++++++++----
>>>> 4 files changed, 23 insertions(+), 35 deletions(-)
>>>>
>>>> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
>>>> index cd8790d..eb76e8e 100644
>>>> --- a/include/linux/bpf.h
>>>> +++ b/include/linux/bpf.h
>>>> @@ -48,8 +48,6 @@ struct bpf_map_ops {
>>>> u32 (*map_fd_sys_lookup_elem)(void *ptr);
>>>> void (*map_seq_show_elem)(struct bpf_map *map, void *key,
>>>> struct seq_file *m);
>>>> - int (*map_check_btf)(const struct bpf_map *map, const struct btf *btf,
>>>> - u32 key_type_id, u32 value_type_id);
>>>> };
>>>>
>>>> struct bpf_map {
>>>> @@ -118,7 +116,7 @@ static inline bool bpf_map_offload_neutral(const struct bpf_map *map)
>>>>
>>>> static inline bool bpf_map_support_seq_show(const struct bpf_map *map)
>>>> {
>>>> - return map->ops->map_seq_show_elem && map->ops->map_check_btf;
>>>> + return map->btf && map->ops->map_seq_show_elem;
>>>> }
>>>>
>>>> extern const struct bpf_map_ops bpf_map_offload_ops;
>>>> diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
>>>> index 2aa55d030..67f0bdf 100644
>>>> --- a/kernel/bpf/arraymap.c
>>>> +++ b/kernel/bpf/arraymap.c
>>>> @@ -358,32 +358,6 @@ static void array_map_seq_show_elem(struct bpf_map *map, void *key,
>>>> rcu_read_unlock();
>>>> }
>>>>
>>>> -static int array_map_check_btf(const struct bpf_map *map, const struct btf *btf,
>>>> - u32 btf_key_id, u32 btf_value_id)
>>>> -{
>>>> - const struct btf_type *key_type, *value_type;
>>>> - u32 key_size, value_size;
>>>> - u32 int_data;
>>>> -
>>>> - key_type = btf_type_id_size(btf, &btf_key_id, &key_size);
>>>> - if (!key_type || BTF_INFO_KIND(key_type->info) != BTF_KIND_INT)
>>>> - return -EINVAL;
>>>> -
>>>> - int_data = *(u32 *)(key_type + 1);
>>>> - /* bpf array can only take a u32 key. This check makes
>>>> - * sure that the btf matches the attr used during map_create.
>>>> - */
>>>> - if (BTF_INT_BITS(int_data) != 32 || key_size != 4 ||
>>>> - BTF_INT_OFFSET(int_data))
>>>> - return -EINVAL;
>>>
>>> I think most of these checks are still necessary for array type.
>>> Relaxing BTF array key from BTF_KIND_INT to, for example, BTF_KIND_ENUM
>>> is probably ok, but key being BTF_KIND_PTR or BTF_KIND_ARRAY doesn't makes sense.
>>
>> Hmm, so on 64 bit archs BTF_KIND_PTR would get rejected for array,
>> on 32 bit it may be allowed due to sizeof(void *) == 4. BTF_KIND_ARRAY
>> could be array of u8 foo[4], for example, or u16 foo[2]. But how would
>> it ultimately be different from e.g. having 'struct a' versus 'struct b'
>> where both are of same size and while actual key has 'struct a', the one
>> who writes the prog resp. loads the BTF into the kernel would lie about
>> it stating it's of type 'struct b' instead? It's basically trusting the
>> app that it advertised sane key types which kernel is propagating back.
>
> for hash map - yes. the kernel cannot yet catch the lie that
> key == 'struct a' that user said in BTF is not what program used
> (which used 'struct b' of the same size).
> Eventually we will annotate all load/store in the program and will
> make sure that memory access match what BTF said.
But in that case, would you reject the program? E.g. from prog point of
view, it's just a buffer of x bytes, so key could be casted to different
struct/types potentially and used for lookup; similar with value if you
would go the route to annotate all access into it. I don't think this
serves as a security feature (since you might as well just load the prog
without BTF just fine), but it can be used to help verifier to perform
rewrites like in tracing for implicit bpf_probe_read() based on member
access. But also in that case, if you might have e.g. stale or wrong BTF
data e.g. of the whole kernel or some application it would follow/walk
that one instead. But such user error would be "acceptable" since it serves
as a hint, roughly similar to (explicitly) walking the data structures
based on the headers today, you do have better control in terms of header
mismatches in that you can ship the BTF directly from the build, but there's
still no guarantee in that sense that you "verified" that these bytes
originally were mapped to struct foo somewhere in a C program.
> For array we can catch the lie today that key is not 4 byte int,
> since it matters from pretty printing point of view.
> If it's PTR or ARRAY or STRUCT, the printer will go nuts.
In that case, would you enforce a hash map key size of 4 also to INT-only
instead of e.g. allowing STRUCT and various others? I don't think it's
that much different from pretty printer PoV (e.g. see bpftool transparently
handling them via btf_dumper_type()), it's some key with 4 bytes of memory
and a type matching these 4 bytes in size for pretty printer.
Thanks,
Daniel
^ permalink raw reply
* [PATCH v0] strparser: remove any offset before parsing messages
From: Dominique Martinet @ 2018-08-09 22:40 UTC (permalink / raw)
To: Doron Roberts-Kedes, Tom Herbert, Dave Watson
Cc: Dominique Martinet, David S. Miller, netdev, linux-kernel
Offset is not well handled by strparser users right now.
Out of the current strparser users, we have:
- tls, that handles offset properly in parse and rcv callbacks
- kcm, that handles offset in rcv but not in parse
- bpf sockmap, that does not seem to handle offset anywhere
Calling pskb_pull() on the skb before parsing ensures that the offset
will be 0 everywhere in practice unless the user modifies it themselves
like tls, as a workaround for the other two protocols.
This fixes a bug whilch can be exhibited by implementing a simpe kcm
parser that looks for the packet size in the first word of the packet,
and sending two such packets in a single write() call on the other side:
the second message will be cut at the length of the first message.
Since this is a stream protocol, all the following messages will also
be corrupt since it will start looking for the next offset at a wrong
position.
Signed-off-by: Dominique Martinet <asmadeus@codewreck.org>
---
Discussions on the bug along with a (bad) reproducer can be found here:
http://lkml.kernel.org/m/20180803182830.GB29193@nautica
(now the problem is better understood though it's much simpler to send
two messages at once than to spam and wait for tcp aggregation to do it)
Two notes:
- I've marked this patch v0 as we could move the pskb_pull() up to
where strp.offset is set, and just always leave it at 0 in the strparser
code.
This will let applications that are fine dealing with a non-zero offset
deal with it as they seem fit (tls writes into the offset and full_len
fields behind the back of the stream parser), while still being safe for
kcm/sockmap
- Even with that modification I'm not totally happy with
single-handedly eating the offset for strparser users which could handle
it, but I'm not really familiar with the cost this really has in
practice...
A better fix would be to handle the offset properly in the callbacks,
but frankly at least for kcm I don't see how (maybe because I'm not
familiar with how bpf programs work)
Another idea I had would be to write flags when registering the protocol
e.g. strp->cb.flags & STRP_CAN_PARSE_WITH_OFFSET or something like that,
but without an idea of the cost of that pull I don't know if it's worth
doing.
Anyway, comments welcome.
net/strparser/strparser.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/net/strparser/strparser.c b/net/strparser/strparser.c
index 625acb27efcc..d7a3b81c3481 100644
--- a/net/strparser/strparser.c
+++ b/net/strparser/strparser.c
@@ -222,6 +222,16 @@ static int __strp_recv(read_descriptor_t *desc, struct sk_buff *orig_skb,
if (!stm->strp.full_len) {
ssize_t len;
+ /* Can only parse if there is no offset */
+ if (unlikely(stm->strp.offset)) {
+ if (!pskb_pull(skb, stm->strp.offset)) {
+ STRP_STATS_INCR(strp->stats.mem_fail);
+ strp_parser_err(strp, -ENOMEM, desc);
+ break;
+ }
+ stm->strp.offset = 0;
+ }
+
len = (*strp->cb.parse_msg)(strp, head);
if (!len) {
@@ -249,8 +259,7 @@ static int __strp_recv(read_descriptor_t *desc, struct sk_buff *orig_skb,
STRP_STATS_INCR(strp->stats.msg_too_big);
strp_parser_err(strp, -EMSGSIZE, desc);
break;
- } else if (len <= (ssize_t)head->len -
- skb->len - stm->strp.offset) {
+ } else if (len <= (ssize_t)head->len - skb->len) {
/* Length must be into new skb (and also
* greater than zero)
*/
--
2.17.1
^ permalink raw reply related
* Re: [PATCH lora-next v2 8/8] net: lora: sx1301: convert driver over to regmap reads and writes
From: Andreas Färber @ 2018-08-09 22:34 UTC (permalink / raw)
To: Ben Whitten
Cc: starnight, hasnain.virk, netdev, liuxuenetmail, shess,
Ben Whitten, Yannick Lanz
In-Reply-To: <1533818018-29005-8-git-send-email-ben.whitten@lairdtech.com>
Am 09.08.2018 um 14:33 schrieb Ben Whitten:
> The reads and writes are replaced with regmap versions and unneeded
> functions, variable, and defines removed.
>
> Signed-off-by: Ben Whitten <ben.whitten@lairdtech.com>
> ---
> drivers/net/lora/sx1301.c | 204 +++++++++++++++-------------------------------
> drivers/net/lora/sx1301.h | 30 +++++++
> 2 files changed, 95 insertions(+), 139 deletions(-)
>
> diff --git a/drivers/net/lora/sx1301.c b/drivers/net/lora/sx1301.c
> index 766df06..4db5a43 100644
> --- a/drivers/net/lora/sx1301.c
> +++ b/drivers/net/lora/sx1301.c
[...]
> @@ -140,50 +115,9 @@ static int sx1301_write(struct sx1301_priv *priv, u8 reg, u8 val)
> return sx1301_write_burst(priv, reg, &val, 1);
> }
_write and _read are now unused, causing warnings. Dropping.
The _burst versions are still in use for firmware load, and I saw a
discussion indicating that regmap is lacking the capability to not
increment the reg for bulk reads at the moment. So we still can't
cleanly switch to regmap entirely and thereby remain bound to SPI.
[...]
> @@ -235,8 +169,8 @@ static int sx1301_radio_spi_transfer_one(struct spi_controller *ctrl,
> {
> struct spi_sx1301 *ssx = spi_controller_get_devdata(ctrl);
> struct sx1301_priv *priv = spi_get_drvdata(ssx->parent);
> - const u8 *tx_buf = xfr->tx_buf;
> - u8 *rx_buf = xfr->rx_buf;
> + const unsigned int *tx_buf = xfr->tx_buf;
> + unsigned int *rx_buf = xfr->rx_buf;
These are wrong both for Little Endian and even worse for Big Endian.
> int ret;
>
> if (xfr->len == 0 || xfr->len > 3)
> @@ -245,13 +179,13 @@ static int sx1301_radio_spi_transfer_one(struct spi_controller *ctrl,
> dev_dbg(&spi->dev, "transferring one (%u)\n", xfr->len);
>
> if (tx_buf) {
> - ret = sx1301_page_write(priv, ssx->page, ssx->regs + REG_RADIO_X_ADDR, tx_buf ? tx_buf[0] : 0);
> + ret = regmap_write(priv->regmap, ssx->regs + REG_RADIO_X_ADDR, tx_buf ? tx_buf[0] : 0);
> if (ret) {
> dev_err(&spi->dev, "SPI radio address write failed\n");
> return ret;
> }
>
> - ret = sx1301_page_write(priv, ssx->page, ssx->regs + REG_RADIO_X_DATA, (tx_buf && xfr->len >= 2) ? tx_buf[1] : 0);
> + ret = regmap_write(priv->regmap, ssx->regs + REG_RADIO_X_DATA, (tx_buf && xfr->len >= 2) ? tx_buf[1] : 0);
> if (ret) {
> dev_err(&spi->dev, "SPI radio data write failed\n");
> return ret;
> @@ -271,7 +205,7 @@ static int sx1301_radio_spi_transfer_one(struct spi_controller *ctrl,
> }
>
> if (rx_buf) {
> - ret = sx1301_page_read(priv, ssx->page, ssx->regs + REG_RADIO_X_DATA_READBACK, &rx_buf[xfr->len - 1]);
> + ret = regmap_read(priv->regmap, ssx->regs + REG_RADIO_X_DATA_READBACK, &rx_buf[xfr->len - 1]);
> if (ret) {
> dev_err(&spi->dev, "SPI radio data read failed\n");
> return ret;
Fixing by adding a local variable instead:
@@ -239,6 +163,7 @@ static int sx1301_radio_spi_transfer_one(struct
spi_controll
er *ctrl,
struct sx1301_priv *priv = netdev_priv(netdev);
const u8 *tx_buf = xfr->tx_buf;
u8 *rx_buf = xfr->rx_buf;
+ unsigned int val;
int ret;
if (xfr->len == 0 || xfr->len > 3)
[...]
@@ -273,27 +198,28 @@ static int sx1301_radio_spi_transfer_one(struct
spi_controller *ctrl,
}
if (rx_buf) {
- ret = sx1301_page_read(priv, ssx->page, ssx->regs +
REG_RADIO_X_DATA_READBACK, &rx_buf[xfr->len - 1]);
+ ret = regmap_read(priv->regmap, ssx->regs +
REG_RADIO_X_DATA_READBACK, &val);
if (ret) {
dev_err(&spi->dev, "SPI radio data read failed\n");
return ret;
}
+ rx_buf[xfr->len - 1] = val & 0xff;
}
return 0;
[...]
> diff --git a/drivers/net/lora/sx1301.h b/drivers/net/lora/sx1301.h
> index 2fc283f..b21e5c6 100644
> --- a/drivers/net/lora/sx1301.h
> +++ b/drivers/net/lora/sx1301.h
> @@ -18,11 +18,41 @@
> /* Page independent */
> #define SX1301_PAGE 0x00
> #define SX1301_VER 0x01
> +#define SX1301_MPA 0x09
Those are the official register names? I find these much harder to read
than my guessed names. Could we keep the long names as aliases?
> +#define SX1301_MPD 0x0A
> +#define SX1301_GEN 0x10
> +#define SX1301_CKEN 0x11
> +#define SX1301_GPSO 0x1C
> +#define SX1301_GPMODE 0x1D
> +#define SX1301_AGCSTS 0x20
>
> #define SX1301_VIRT_BASE 0x100
> #define SX1301_PAGE_LEN 0x80
> #define SX1301_PAGE_BASE(n) (SX1301_VIRT_BASE + (SX1301_PAGE_LEN * n))
>
> +/* Page 0 */
> +#define SX1301_CHRS (SX1301_PAGE_BASE(0) + 0x23)
> +#define SX1301_FORCE_CTRL (SX1301_PAGE_BASE(0) + 0x69)
> +#define SX1301_MCU_CTRL (SX1301_PAGE_BASE(0) + 0x6A)
> +
> +/* Page 2 */
> +#define SX1301_RADIO_A_SPI_DATA (SX1301_PAGE_BASE(2) + 0x21)
> +#define SX1301_RADIO_A_SPI_DATA_RB (SX1301_PAGE_BASE(2) + 0x22)
> +#define SX1301_RADIO_A_SPI_ADDR (SX1301_PAGE_BASE(2) + 0x23)
> +#define SX1301_RADIO_A_SPI_CS (SX1301_PAGE_BASE(2) + 0x25)
> +#define SX1301_RADIO_B_SPI_DATA (SX1301_PAGE_BASE(2) + 0x26)
> +#define SX1301_RADIO_B_SPI_DATA_RB (SX1301_PAGE_BASE(2) + 0x27)
> +#define SX1301_RADIO_B_SPI_ADDR (SX1301_PAGE_BASE(2) + 0x28)
> +#define SX1301_RADIO_B_SPI_CS (SX1301_PAGE_BASE(2) + 0x2A)
> +#define SX1301_RADIO_CFG (SX1301_PAGE_BASE(2) + 0x2B)
> +#define SX1301_DBG_ARB_MCU_RAM_DATA (SX1301_PAGE_BASE(2) + 0x40)
> +#define SX1301_DBG_AGC_MCU_RAM_DATA (SX1301_PAGE_BASE(2) + 0x41)
> +#define SX1301_DBG_ARB_MCU_RAM_ADDR (SX1301_PAGE_BASE(2) + 0x50)
> +#define SX1301_DBG_AGC_MCU_RAM_ADDR (SX1301_PAGE_BASE(2) + 0x51)
> +
> +/* Page 3 */
> +#define SX1301_EMERGENCY_FORCE_HOST_CTRL (SX1301_PAGE_BASE(3) + 0x7F)
> +
> #define SX1301_MAX_REGISTER (SX1301_PAGE_BASE(3) + 0x7F)
>
> #endif
Applying so that we can continue based on regmap.
Regards,
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
^ permalink raw reply
* Re: [PATCH net-next v2] net: allow to call netif_reset_xps_queues() under cpus_read_lock
From: Michael S. Tsirkin @ 2018-08-09 22:13 UTC (permalink / raw)
To: Andrei Vagin
Cc: David S. Miller, netdev, Andrei Vagin, Nambiar, Amritha,
Jason Wang
In-Reply-To: <20180809030735.16173-1-avagin@openvz.org>
On Wed, Aug 08, 2018 at 08:07:35PM -0700, Andrei Vagin wrote:
> From: Andrei Vagin <avagin@gmail.com>
>
> The definition of static_key_slow_inc() has cpus_read_lock in place. In the
> virtio_net driver, XPS queues are initialized after setting the queue:cpu
> affinity in virtnet_set_affinity() which is already protected within
> cpus_read_lock. Lockdep prints a warning when we are trying to acquire
> cpus_read_lock when it is already held.
>
> This patch adds an ability to call __netif_set_xps_queue under
> cpus_read_lock().
>
> ============================================
> WARNING: possible recursive locking detected
> 4.18.0-rc3-next-20180703+ #1 Not tainted
> --------------------------------------------
> swapper/0/1 is trying to acquire lock:
> 00000000cf973d46 (cpu_hotplug_lock.rw_sem){++++}, at: static_key_slow_inc+0xe/0x20
>
> but task is already holding lock:
> 00000000cf973d46 (cpu_hotplug_lock.rw_sem){++++}, at: init_vqs+0x513/0x5a0
>
> other info that might help us debug this:
> Possible unsafe locking scenario:
>
> CPU0
> ----
> lock(cpu_hotplug_lock.rw_sem);
> lock(cpu_hotplug_lock.rw_sem);
>
> *** DEADLOCK ***
>
> May be due to missing lock nesting notation
>
> 3 locks held by swapper/0/1:
> #0: 00000000244bc7da (&dev->mutex){....}, at: __driver_attach+0x5a/0x110
> #1: 00000000cf973d46 (cpu_hotplug_lock.rw_sem){++++}, at: init_vqs+0x513/0x5a0
> #2: 000000005cd8463f (xps_map_mutex){+.+.}, at: __netif_set_xps_queue+0x8d/0xc60
>
> v2: move cpus_read_lock() out of __netif_set_xps_queue()
FYI you change log should go after -- below, not before it.
> Cc: "Nambiar, Amritha" <amritha.nambiar@intel.com>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Jason Wang <jasowang@redhat.com>
> Fixes: 8af2c06ff4b1 ("net-sysfs: Add interface for Rx queue(s) map per Tx queue")
>
> Signed-off-by: Andrei Vagin <avagin@gmail.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> drivers/net/virtio_net.c | 4 +++-
> net/core/dev.c | 20 +++++++++++++++-----
> net/core/net-sysfs.c | 4 ++++
> 3 files changed, 22 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 62311dde6e71..39a7f4452587 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -1903,9 +1903,11 @@ static void virtnet_set_affinity(struct virtnet_info *vi)
>
> i = 0;
> for_each_online_cpu(cpu) {
> + const unsigned long *mask = cpumask_bits(cpumask_of(cpu));
> +
> virtqueue_set_affinity(vi->rq[i].vq, cpu);
> virtqueue_set_affinity(vi->sq[i].vq, cpu);
> - netif_set_xps_queue(vi->dev, cpumask_of(cpu), i);
> + __netif_set_xps_queue(vi->dev, mask, i, false);
> i++;
> }
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index f68122f0ab02..325fc5088370 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -2176,6 +2176,7 @@ static void netif_reset_xps_queues(struct net_device *dev, u16 offset,
> if (!static_key_false(&xps_needed))
> return;
>
> + cpus_read_lock();
> mutex_lock(&xps_map_mutex);
>
> if (static_key_false(&xps_rxqs_needed)) {
> @@ -2199,10 +2200,11 @@ static void netif_reset_xps_queues(struct net_device *dev, u16 offset,
>
> out_no_maps:
> if (static_key_enabled(&xps_rxqs_needed))
> - static_key_slow_dec(&xps_rxqs_needed);
> + static_key_slow_dec_cpuslocked(&xps_rxqs_needed);
>
> - static_key_slow_dec(&xps_needed);
> + static_key_slow_dec_cpuslocked(&xps_needed);
> mutex_unlock(&xps_map_mutex);
> + cpus_read_unlock();
> }
>
> static void netif_reset_xps_queues_gt(struct net_device *dev, u16 index)
> @@ -2250,6 +2252,7 @@ static struct xps_map *expand_xps_map(struct xps_map *map, int attr_index,
> return new_map;
> }
>
> +/* Must be called under cpus_read_lock */
> int __netif_set_xps_queue(struct net_device *dev, const unsigned long *mask,
> u16 index, bool is_rxqs_map)
> {
> @@ -2317,9 +2320,9 @@ int __netif_set_xps_queue(struct net_device *dev, const unsigned long *mask,
> if (!new_dev_maps)
> goto out_no_new_maps;
>
> - static_key_slow_inc(&xps_needed);
> + static_key_slow_inc_cpuslocked(&xps_needed);
> if (is_rxqs_map)
> - static_key_slow_inc(&xps_rxqs_needed);
> + static_key_slow_inc_cpuslocked(&xps_rxqs_needed);
>
> for (j = -1; j = netif_attrmask_next(j, possible_mask, nr_ids),
> j < nr_ids;) {
> @@ -2448,11 +2451,18 @@ int __netif_set_xps_queue(struct net_device *dev, const unsigned long *mask,
> kfree(new_dev_maps);
> return -ENOMEM;
> }
> +EXPORT_SYMBOL_GPL(__netif_set_xps_queue);
>
> int netif_set_xps_queue(struct net_device *dev, const struct cpumask *mask,
> u16 index)
> {
> - return __netif_set_xps_queue(dev, cpumask_bits(mask), index, false);
> + int ret;
> +
> + cpus_read_lock();
> + ret = __netif_set_xps_queue(dev, cpumask_bits(mask), index, false);
> + cpus_read_unlock();
> +
> + return ret;
> }
> EXPORT_SYMBOL(netif_set_xps_queue);
>
> diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
> index 0a95bcf64cdc..bd67c4d0fcfd 100644
> --- a/net/core/net-sysfs.c
> +++ b/net/core/net-sysfs.c
> @@ -26,6 +26,7 @@
> #include <linux/pm_runtime.h>
> #include <linux/of.h>
> #include <linux/of_net.h>
> +#include <linux/cpu.h>
>
> #include "net-sysfs.h"
>
> @@ -1400,7 +1401,10 @@ static ssize_t xps_rxqs_store(struct netdev_queue *queue, const char *buf,
> return err;
> }
>
> + cpus_read_lock();
> err = __netif_set_xps_queue(dev, mask, index, true);
> + cpus_read_unlock();
> +
> kfree(mask);
> return err ? : len;
> }
> --
> 2.17.1
^ permalink raw reply
* Re: KCM - recvmsg() mangles packets?
From: Dominique Martinet @ 2018-08-09 22:06 UTC (permalink / raw)
To: Tom Herbert; +Cc: Linux Kernel Network Developers
In-Reply-To: <CAPDqMeptLoLZ2RU+T-d7YwkO2AeKV+nfow4C-LqqeZd9mo-vMg@mail.gmail.com>
Tom Herbert wrote on Thu, Aug 09, 2018:
> > diff --git a/net/strparser/strparser.c b/net/strparser/strparser.c
> > index 625acb27efcc..348ff5945591 100644
> > --- a/net/strparser/strparser.c
> > +++ b/net/strparser/strparser.c
> > @@ -222,6 +222,16 @@ static int __strp_recv(read_descriptor_t *desc, struct sk_buff *orig_skb,
> > if (!stm->strp.full_len) {
> > ssize_t len;
> >
> > + /* Can only parse if there is no offset */
> > + if (unlikely(stm->strp.offset)) {
> > + if (!pskb_pull(skb, stm->strp.offset)) {
> > + STRP_STATS_INCR(strp->stats.mem_fail);
> > + strp_parser_err(strp, -ENOMEM, desc);
> > + break;
> > + }
> > + stm->strp.offset = 0;
> > + }
> > +
>
> Seems okay to me for a fix.
Hmm, if you say so, I'll send this as a patch for broader comments right
away.
> Looks like strp.offset is only set in one place and read in one
> place. With this pull maybe that just can go away?
Good point, when strp.offset is set the full_len is also being init'd so
we will necessarily do the pull next...
But the way tls uses strparser is also kind of weird, since they modify
the strp_msg's offset and full_len, I wouldn't want to assume we can't
have full_len == 0 *again* later with a non zero offset...
On the other hand they do handle non-zero offset in their parse function
so they'd be ok with that... Ultimately it's probably closer to a design
choice than anything else.
I'll still send a v0 of the patch as is, because I feel it's easier to
understand that we pull because the existing parse_msg functions do not
handle it properly, and will write a note that I intend to move it up a
few lines as a comment.
Thanks,
--
Dominique Martinet
^ permalink raw reply
* Re: [PATCH lora-next v2 7/8] net: lora: sx1301: add initial registration for regmap
From: Andreas Färber @ 2018-08-09 21:58 UTC (permalink / raw)
To: Ben Whitten
Cc: starnight, hasnain.virk, netdev, liuxuenetmail, shess,
Ben Whitten
In-Reply-To: <1533818018-29005-7-git-send-email-ben.whitten@lairdtech.com>
Am 09.08.2018 um 14:33 schrieb Ben Whitten:
> The register and bit-field definitions are taken from the SX1301
> datasheet version 2.01 dated June 2014 with the revision information
> 'First released version'.
>
> The reset state and RW capability of each field is not reflected in this
> patch however from the datasheet:
> "Bits and registers that are not documented are reserved. They may
> include calibration values. It is important not to modify these bits and
> registers. If specific bits must be changed in a register with reserved
> bits, the register must be read first, specific bits modified while
> masking reserved bits and then the register can be written."
>
> Then goes on to state:
> "Reserved bits should be written with their reset state, they may be
> read different states."
>
> Caching is currently disabled.
>
> The version is read back using regmap_read to verify regmap operation,
> in doing so needs to be moved after priv and regmap allocation.
>
> Further registers or fields are added as they are required in conversion.
>
> Signed-off-by: Ben Whitten <ben.whitten@lairdtech.com>
> ---
> drivers/net/lora/Kconfig | 1 +
> drivers/net/lora/sx1301.c | 46 ++++++++++++++++++++++++++++++++++++++++++----
> drivers/net/lora/sx1301.h | 10 ++++++++++
> 3 files changed, 53 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/lora/Kconfig b/drivers/net/lora/Kconfig
> index bb57a01..79d23f2 100644
> --- a/drivers/net/lora/Kconfig
> +++ b/drivers/net/lora/Kconfig
> @@ -49,6 +49,7 @@ config LORA_SX1301
> tristate "Semtech SX1301 SPI driver"
> default y
> depends on SPI
> + select REGMAP_SPI
> help
> Semtech SX1301
>
> diff --git a/drivers/net/lora/sx1301.c b/drivers/net/lora/sx1301.c
> index 8e81179..766df06 100644
> --- a/drivers/net/lora/sx1301.c
> +++ b/drivers/net/lora/sx1301.c
> @@ -20,11 +20,11 @@
> #include <linux/of_gpio.h>
> #include <linux/lora/dev.h>
> #include <linux/spi/spi.h>
> +#include <linux/regmap.h>
Misordered.
>
> #include "sx1301.h"
>
> #define REG_PAGE_RESET 0
> -#define REG_VERSION 1
> #define REG_MCU_PROM_ADDR 9
> #define REG_MCU_PROM_DATA 10
> #define REG_GPIO_SELECT_INPUT 27
> @@ -68,6 +68,35 @@
>
> #define REG_EMERGENCY_FORCE_HOST_CTRL BIT(0)
>
> +static const struct regmap_range_cfg sx1301_ranges[] = {
Let's rename to _regmap_ranges for consistency.
> @@ -81,6 +110,7 @@ struct sx1301_priv {
> struct gpio_desc *rst_gpio;
> u8 cur_page;
> struct spi_controller *radio_a_ctrl, *radio_b_ctrl;
> + struct regmap *regmap;
Note: We need a consistent style. Either whitespace or tabs, not both
depending on author. Same in an earlier patch. Problem with tabs is that
at some point it's always one tab too little, but we can try it.
Applied.
Thanks,
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
^ permalink raw reply
* Re: [PATCH bpf-next] bpf: enable btf for use in all maps
From: Alexei Starovoitov @ 2018-08-09 21:44 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: ast, netdev, yhs
In-Reply-To: <d851548d-e027-57ee-55c9-a61c48ffcf15@iogearbox.net>
On Thu, Aug 09, 2018 at 11:30:52PM +0200, Daniel Borkmann wrote:
> On 08/09/2018 11:14 PM, Alexei Starovoitov wrote:
> > On Thu, Aug 09, 2018 at 09:42:20PM +0200, Daniel Borkmann wrote:
> >> Commit a26ca7c982cb ("bpf: btf: Add pretty print support to
> >> the basic arraymap") enabled support for BTF and dumping via
> >> BPF fs for arraymap. However, both can be decoupled from each
> >> other such that all BPF maps can be supported for attaching
> >> BTF key/value information, while not all maps necessarily
> >> need to dump via map_seq_show_elem() callback.
> >>
> >> The check in array_map_check_btf() can be generalized as
> >> ultimatively the key and value size is the only contraint
> >> that needs to match for the map. The fact that the key needs
> >> to be of type int is optional; it could be any data type as
> >> long as it matches the 4 byte key size, just like hash table
> >> key or others could be of any data type as well.
> >>
> >> Minimal example of a hash table dump which then works out
> >> of the box for bpftool:
> >>
> >> # bpftool map dump id 19
> >> [{
> >> "key": {
> >> "": {
> >> "vip": 0,
> >> "vipv6": []
> >> },
> >> "port": 0,
> >> "family": 0,
> >> "proto": 0
> >> },
> >> "value": {
> >> "flags": 0,
> >> "vip_num": 0
> >> }
> >> }
> >> ]
> >>
> >> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> >> Cc: Yonghong Song <yhs@fb.com>
> >> ---
> >> include/linux/bpf.h | 4 +---
> >> kernel/bpf/arraymap.c | 27 ---------------------------
> >> kernel/bpf/inode.c | 3 ++-
> >> kernel/bpf/syscall.c | 24 ++++++++++++++++++++----
> >> 4 files changed, 23 insertions(+), 35 deletions(-)
> >>
> >> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> >> index cd8790d..eb76e8e 100644
> >> --- a/include/linux/bpf.h
> >> +++ b/include/linux/bpf.h
> >> @@ -48,8 +48,6 @@ struct bpf_map_ops {
> >> u32 (*map_fd_sys_lookup_elem)(void *ptr);
> >> void (*map_seq_show_elem)(struct bpf_map *map, void *key,
> >> struct seq_file *m);
> >> - int (*map_check_btf)(const struct bpf_map *map, const struct btf *btf,
> >> - u32 key_type_id, u32 value_type_id);
> >> };
> >>
> >> struct bpf_map {
> >> @@ -118,7 +116,7 @@ static inline bool bpf_map_offload_neutral(const struct bpf_map *map)
> >>
> >> static inline bool bpf_map_support_seq_show(const struct bpf_map *map)
> >> {
> >> - return map->ops->map_seq_show_elem && map->ops->map_check_btf;
> >> + return map->btf && map->ops->map_seq_show_elem;
> >> }
> >>
> >> extern const struct bpf_map_ops bpf_map_offload_ops;
> >> diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
> >> index 2aa55d030..67f0bdf 100644
> >> --- a/kernel/bpf/arraymap.c
> >> +++ b/kernel/bpf/arraymap.c
> >> @@ -358,32 +358,6 @@ static void array_map_seq_show_elem(struct bpf_map *map, void *key,
> >> rcu_read_unlock();
> >> }
> >>
> >> -static int array_map_check_btf(const struct bpf_map *map, const struct btf *btf,
> >> - u32 btf_key_id, u32 btf_value_id)
> >> -{
> >> - const struct btf_type *key_type, *value_type;
> >> - u32 key_size, value_size;
> >> - u32 int_data;
> >> -
> >> - key_type = btf_type_id_size(btf, &btf_key_id, &key_size);
> >> - if (!key_type || BTF_INFO_KIND(key_type->info) != BTF_KIND_INT)
> >> - return -EINVAL;
> >> -
> >> - int_data = *(u32 *)(key_type + 1);
> >> - /* bpf array can only take a u32 key. This check makes
> >> - * sure that the btf matches the attr used during map_create.
> >> - */
> >> - if (BTF_INT_BITS(int_data) != 32 || key_size != 4 ||
> >> - BTF_INT_OFFSET(int_data))
> >> - return -EINVAL;
> >
> > I think most of these checks are still necessary for array type.
> > Relaxing BTF array key from BTF_KIND_INT to, for example, BTF_KIND_ENUM
> > is probably ok, but key being BTF_KIND_PTR or BTF_KIND_ARRAY doesn't makes sense.
>
> Hmm, so on 64 bit archs BTF_KIND_PTR would get rejected for array,
> on 32 bit it may be allowed due to sizeof(void *) == 4. BTF_KIND_ARRAY
> could be array of u8 foo[4], for example, or u16 foo[2]. But how would
> it ultimately be different from e.g. having 'struct a' versus 'struct b'
> where both are of same size and while actual key has 'struct a', the one
> who writes the prog resp. loads the BTF into the kernel would lie about
> it stating it's of type 'struct b' instead? It's basically trusting the
> app that it advertised sane key types which kernel is propagating back.
for hash map - yes. the kernel cannot yet catch the lie that
key == 'struct a' that user said in BTF is not what program used
(which used 'struct b' of the same size).
Eventually we will annotate all load/store in the program and will
make sure that memory access match what BTF said.
For array we can catch the lie today that key is not 4 byte int,
since it matters from pretty printing point of view.
If it's PTR or ARRAY or STRUCT, the printer will go nuts.
When userspace can trust kernel that array key is u32 it can print
int arr[10];
just like gdb does:
(gdb) p arr
$1 = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0}
(gdb) ptype arr
type = int [10]
What user printer suppose to do if kernel says that key=PTR or, worse, key=STRUCT ?
I cannot think of sane way of printing such array.
Even key=ENUM is not trivial to print, but I think it can be useful and
practical to use ENUM as a key, but for now I'd stick to INT only
like the check does today.
^ permalink raw reply
* Re: [Patch net-next] net_sched: fix a potential out-of-bound access
From: Cong Wang @ 2018-08-09 21:43 UTC (permalink / raw)
To: Vlad Buslov; +Cc: Linux Kernel Network Developers, Jiri Pirko
In-Reply-To: <vbflg9g2dx5.fsf@reg-r-vrt-018-180.mtr.labs.mlnx>
On Thu, Aug 9, 2018 at 12:32 AM Vlad Buslov <vladbu@mellanox.com> wrote:
>
> Before version V5 of my action API patchset this functionality was
> implemented in exactly the same way as in your patch. Unfortunately, it
> has a double-free bug. The problem is that if you have multiple
> actions(N) being deleted, and deleted succeeded for first K actions,
> this implementation will try to delete all N actions second time
> (including first K actions that were already deleted). That is why I
> added 'acts_deleted' variable that tracks actual amount of actions that
> were deleted successfully, and only delete last N-K actions in case of
> error.
Interesting, I didn't notice you call it for tcf_del_notify()'s failure too.
But this is easy to resolve, we can just set succeeded ones to NULL
and teach tcf_action_put_many() to scan the whole array but
skip NULL's.
>
> In order to fix that issue I did following code changes in V5:
> - Added 'acts_deleted' variable to delete only actions [K, N) in case of
> error.
> - Extended 'actions' array size by one to ensure that it always ends
> with NULL pointer.
Oh, I see, this is not how we use C, you can at least rollback
by passing acts_deleted as a parameter as the start of the array.
You picked the most confusing way to handle it.
I will send an updated patch.
^ permalink raw reply
* Re: [PATCH bpf-next] bpf: enable btf for use in all maps
From: Daniel Borkmann @ 2018-08-09 21:30 UTC (permalink / raw)
To: Alexei Starovoitov; +Cc: ast, netdev, yhs
In-Reply-To: <20180809211416.oznmx5jlnbagkk3w@ast-mbp>
On 08/09/2018 11:14 PM, Alexei Starovoitov wrote:
> On Thu, Aug 09, 2018 at 09:42:20PM +0200, Daniel Borkmann wrote:
>> Commit a26ca7c982cb ("bpf: btf: Add pretty print support to
>> the basic arraymap") enabled support for BTF and dumping via
>> BPF fs for arraymap. However, both can be decoupled from each
>> other such that all BPF maps can be supported for attaching
>> BTF key/value information, while not all maps necessarily
>> need to dump via map_seq_show_elem() callback.
>>
>> The check in array_map_check_btf() can be generalized as
>> ultimatively the key and value size is the only contraint
>> that needs to match for the map. The fact that the key needs
>> to be of type int is optional; it could be any data type as
>> long as it matches the 4 byte key size, just like hash table
>> key or others could be of any data type as well.
>>
>> Minimal example of a hash table dump which then works out
>> of the box for bpftool:
>>
>> # bpftool map dump id 19
>> [{
>> "key": {
>> "": {
>> "vip": 0,
>> "vipv6": []
>> },
>> "port": 0,
>> "family": 0,
>> "proto": 0
>> },
>> "value": {
>> "flags": 0,
>> "vip_num": 0
>> }
>> }
>> ]
>>
>> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
>> Cc: Yonghong Song <yhs@fb.com>
>> ---
>> include/linux/bpf.h | 4 +---
>> kernel/bpf/arraymap.c | 27 ---------------------------
>> kernel/bpf/inode.c | 3 ++-
>> kernel/bpf/syscall.c | 24 ++++++++++++++++++++----
>> 4 files changed, 23 insertions(+), 35 deletions(-)
>>
>> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
>> index cd8790d..eb76e8e 100644
>> --- a/include/linux/bpf.h
>> +++ b/include/linux/bpf.h
>> @@ -48,8 +48,6 @@ struct bpf_map_ops {
>> u32 (*map_fd_sys_lookup_elem)(void *ptr);
>> void (*map_seq_show_elem)(struct bpf_map *map, void *key,
>> struct seq_file *m);
>> - int (*map_check_btf)(const struct bpf_map *map, const struct btf *btf,
>> - u32 key_type_id, u32 value_type_id);
>> };
>>
>> struct bpf_map {
>> @@ -118,7 +116,7 @@ static inline bool bpf_map_offload_neutral(const struct bpf_map *map)
>>
>> static inline bool bpf_map_support_seq_show(const struct bpf_map *map)
>> {
>> - return map->ops->map_seq_show_elem && map->ops->map_check_btf;
>> + return map->btf && map->ops->map_seq_show_elem;
>> }
>>
>> extern const struct bpf_map_ops bpf_map_offload_ops;
>> diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
>> index 2aa55d030..67f0bdf 100644
>> --- a/kernel/bpf/arraymap.c
>> +++ b/kernel/bpf/arraymap.c
>> @@ -358,32 +358,6 @@ static void array_map_seq_show_elem(struct bpf_map *map, void *key,
>> rcu_read_unlock();
>> }
>>
>> -static int array_map_check_btf(const struct bpf_map *map, const struct btf *btf,
>> - u32 btf_key_id, u32 btf_value_id)
>> -{
>> - const struct btf_type *key_type, *value_type;
>> - u32 key_size, value_size;
>> - u32 int_data;
>> -
>> - key_type = btf_type_id_size(btf, &btf_key_id, &key_size);
>> - if (!key_type || BTF_INFO_KIND(key_type->info) != BTF_KIND_INT)
>> - return -EINVAL;
>> -
>> - int_data = *(u32 *)(key_type + 1);
>> - /* bpf array can only take a u32 key. This check makes
>> - * sure that the btf matches the attr used during map_create.
>> - */
>> - if (BTF_INT_BITS(int_data) != 32 || key_size != 4 ||
>> - BTF_INT_OFFSET(int_data))
>> - return -EINVAL;
>
> I think most of these checks are still necessary for array type.
> Relaxing BTF array key from BTF_KIND_INT to, for example, BTF_KIND_ENUM
> is probably ok, but key being BTF_KIND_PTR or BTF_KIND_ARRAY doesn't makes sense.
Hmm, so on 64 bit archs BTF_KIND_PTR would get rejected for array,
on 32 bit it may be allowed due to sizeof(void *) == 4. BTF_KIND_ARRAY
could be array of u8 foo[4], for example, or u16 foo[2]. But how would
it ultimately be different from e.g. having 'struct a' versus 'struct b'
where both are of same size and while actual key has 'struct a', the one
who writes the prog resp. loads the BTF into the kernel would lie about
it stating it's of type 'struct b' instead? It's basically trusting the
app that it advertised sane key types which kernel is propagating back.
Thanks,
Daniel
^ permalink raw reply
* Re: [PATCH net-next] cxgb4: update 1.20.8.0 as the latest firmware supported
From: David Miller @ 2018-08-09 21:27 UTC (permalink / raw)
To: ganeshgr; +Cc: netdev, nirranjan, indranil, dt
In-Reply-To: <1533798123-21071-1-git-send-email-ganeshgr@chelsio.com>
From: Ganesh Goudar <ganeshgr@chelsio.com>
Date: Thu, 9 Aug 2018 12:32:03 +0530
> Change t4fw_version.h to update latest firmware version
> number to 1.20.8.0.
>
> Signed-off-by: Ganesh Goudar <ganeshgr@chelsio.com>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH net-next v2] net: allow to call netif_reset_xps_queues() under cpus_read_lock
From: David Miller @ 2018-08-09 21:26 UTC (permalink / raw)
To: avagin; +Cc: netdev, avagin, amritha.nambiar, mst, jasowang
In-Reply-To: <20180809030735.16173-1-avagin@openvz.org>
From: Andrei Vagin <avagin@openvz.org>
Date: Wed, 8 Aug 2018 20:07:35 -0700
> From: Andrei Vagin <avagin@gmail.com>
>
> The definition of static_key_slow_inc() has cpus_read_lock in place. In the
> virtio_net driver, XPS queues are initialized after setting the queue:cpu
> affinity in virtnet_set_affinity() which is already protected within
> cpus_read_lock. Lockdep prints a warning when we are trying to acquire
> cpus_read_lock when it is already held.
>
> This patch adds an ability to call __netif_set_xps_queue under
> cpus_read_lock().
...
Applied, thank you.
^ permalink raw reply
* Re: [PATCH net-next v2 1/1] net/tls: Combined memory allocation for decryption request
From: David Miller @ 2018-08-09 21:23 UTC (permalink / raw)
To: vakul.garg; +Cc: netdev, borisp, aviadye, davejwatson
In-Reply-To: <20180808232623.18075-2-vakul.garg@nxp.com>
From: Vakul Garg <vakul.garg@nxp.com>
Date: Thu, 9 Aug 2018 04:56:23 +0530
> For preparing decryption request, several memory chunks are required
> (aead_req, sgin, sgout, iv, aad). For submitting the decrypt request to
> an accelerator, it is required that the buffers which are read by the
> accelerator must be dma-able and not come from stack. The buffers for
> aad and iv can be separately kmalloced each, but it is inefficient.
> This patch does a combined allocation for preparing decryption request
> and then segments into aead_req || sgin || sgout || iv || aad.
>
> Signed-off-by: Vakul Garg <vakul.garg@nxp.com>
> ---
> This patch needs to be applied over Doron Roberts-Kedes's patch.
> net/tls: Calculate nsg for zerocopy path without skb_cow_data.
That's going to have many changes, I gave feedback on it yesterday.
Please do not post patches which have pre-requisites which are in
the process of changing or similar as that makes a lot more work
for me and you are also asking people to review changes on top
of code which is going to change.
Thanks.
^ permalink raw reply
* Re: [PATCH lora-next v2 3/8] net: lora: sx1301: convert to passing priv data throughout
From: Andreas Färber @ 2018-08-09 21:21 UTC (permalink / raw)
To: Ben Whitten
Cc: 潘建宏, hasnain.virk, netdev, Xue Liu, shess,
Ben Whitten, Yannick Lanz
In-Reply-To: <CAF3==is4CLr5faEyzB2ADbsDXWD8A4eca73+CHZvPTkY81yqDQ@mail.gmail.com>
Am 09.08.2018 um 23:06 schrieb Ben Whitten:
> On Thu, 9 Aug 2018 at 21:43, Andreas Färber <afaerber@suse.de> wrote:
>> Am 09.08.2018 um 14:33 schrieb Ben Whitten:
>>> @@ -654,22 +646,35 @@ static int sx1301_probe(struct spi_device *spi)
>>> priv->rst_gpio = rst;
>>> priv->cur_page = 0xff;
>>>
>>> - spi_set_drvdata(spi, netdev);
>>> + spi_set_drvdata(spi, priv);
>>
>> This change seems unnecessary and counter-productive for unregistration.
>>
>> Otherwise applying.
>
> This is actually pretty critical, as it stands with the two spi masters we use
> spi_get_drvdata on the parent device of the controller to recover the priv
> struct for regmap.
>
> We may have to include the netdev in the priv data, or do a container_of
> dance to recover netdev in unregistration.
> That said if we wrap things in devm then really our remove function could
> be empty, as we have done with the allocation.
Thanks for quickly noticing. This should compensate:
diff --git a/drivers/net/lora/sx1301.c b/drivers/net/lora/sx1301.c
index 0ba841f8e7cd..43cd2308e41c 100644
--- a/drivers/net/lora/sx1301.c
+++ b/drivers/net/lora/sx1301.c
@@ -164,7 +164,8 @@ static int sx1301_soft_reset(struct sx1301_priv *priv)
static int sx1301_radio_set_cs(struct spi_controller *ctrl, bool enable)
{
struct spi_sx1301 *ssx = spi_controller_get_devdata(ctrl);
- struct sx1301_priv *priv = spi_get_drvdata(ssx->parent);
+ struct net_device *netdev = spi_get_drvdata(ssx->parent);
+ struct sx1301_priv *priv = netdev_priv(netdev);
u8 cs;
int ret;
@@ -204,7 +205,8 @@ static int sx1301_radio_spi_transfer_one(struct
spi_controller *ctrl,
struct spi_device *spi, struct spi_transfer *xfr)
{
struct spi_sx1301 *ssx = spi_controller_get_devdata(ctrl);
- struct sx1301_priv *priv = spi_get_drvdata(ssx->parent);
+ struct net_device *netdev = spi_get_drvdata(ssx->parent);
+ struct sx1301_priv *priv = netdev_priv(netdev);
const u8 *tx_buf = xfr->tx_buf;
u8 *rx_buf = xfr->rx_buf;
int ret;
Regards,
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
^ permalink raw reply related
* Re: [PATCH v2 net-next] net: phy: sfp: print debug message with text, not numbers
From: David Miller @ 2018-08-09 21:20 UTC (permalink / raw)
To: andrew; +Cc: netdev, rmk+kernel, f.fainelli
In-Reply-To: <1533819620-19535-1-git-send-email-andrew@lunn.ch>
From: Andrew Lunn <andrew@lunn.ch>
Date: Thu, 9 Aug 2018 15:00:20 +0200
> Convert the state numbers, device state, etc from numbers to strings
> when printing debug messages.
>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> Acked-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> v2: Fixed typo in subject line.
> Add Acked-by from Florian
Grrr, I committed and pushed out the version with the Subject header
typo.
I could revert and apply this one but that would just make things
look worse.
Sorry, will be more careful next time!
^ permalink raw reply
* Re: [PATCH] net: dsa: rtl8366rb: Support port 4 (WAN)
From: David Miller @ 2018-08-09 21:15 UTC (permalink / raw)
To: linus.walleij
Cc: andrew, vivien.didelot, f.fainelli, netdev, openwrt-devel,
lede-dev
In-Reply-To: <20180808123855.18729-1-linus.walleij@linaro.org>
From: Linus Walleij <linus.walleij@linaro.org>
Date: Wed, 8 Aug 2018 14:38:55 +0200
> The totally undocumented IO mode needs to be set to enumerator
> 0 to enable port 4 also known as WAN in most configurations,
> for ordinary traffic. The 3 bits in the register come up as
> 010 after reset, but need to be set to 000.
>
> The Realtek source code contains a name for these bits, but
> no explanation of what the 8 different IO modes may be.
>
> Set it to zero for the time being and drop a comment so
> people know what is going on if they run into trouble. This
> "mode zero" works fine with the D-Link DIR-685 with
> RTL8366RB.
>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Applied to net-next, thank you.
^ permalink raw reply
* Re: [PATCH bpf-next] bpf: enable btf for use in all maps
From: Alexei Starovoitov @ 2018-08-09 21:14 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: ast, netdev, yhs
In-Reply-To: <20180809194220.17484-1-daniel@iogearbox.net>
On Thu, Aug 09, 2018 at 09:42:20PM +0200, Daniel Borkmann wrote:
> Commit a26ca7c982cb ("bpf: btf: Add pretty print support to
> the basic arraymap") enabled support for BTF and dumping via
> BPF fs for arraymap. However, both can be decoupled from each
> other such that all BPF maps can be supported for attaching
> BTF key/value information, while not all maps necessarily
> need to dump via map_seq_show_elem() callback.
>
> The check in array_map_check_btf() can be generalized as
> ultimatively the key and value size is the only contraint
> that needs to match for the map. The fact that the key needs
> to be of type int is optional; it could be any data type as
> long as it matches the 4 byte key size, just like hash table
> key or others could be of any data type as well.
>
> Minimal example of a hash table dump which then works out
> of the box for bpftool:
>
> # bpftool map dump id 19
> [{
> "key": {
> "": {
> "vip": 0,
> "vipv6": []
> },
> "port": 0,
> "family": 0,
> "proto": 0
> },
> "value": {
> "flags": 0,
> "vip_num": 0
> }
> }
> ]
>
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> Cc: Yonghong Song <yhs@fb.com>
> ---
> include/linux/bpf.h | 4 +---
> kernel/bpf/arraymap.c | 27 ---------------------------
> kernel/bpf/inode.c | 3 ++-
> kernel/bpf/syscall.c | 24 ++++++++++++++++++++----
> 4 files changed, 23 insertions(+), 35 deletions(-)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index cd8790d..eb76e8e 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -48,8 +48,6 @@ struct bpf_map_ops {
> u32 (*map_fd_sys_lookup_elem)(void *ptr);
> void (*map_seq_show_elem)(struct bpf_map *map, void *key,
> struct seq_file *m);
> - int (*map_check_btf)(const struct bpf_map *map, const struct btf *btf,
> - u32 key_type_id, u32 value_type_id);
> };
>
> struct bpf_map {
> @@ -118,7 +116,7 @@ static inline bool bpf_map_offload_neutral(const struct bpf_map *map)
>
> static inline bool bpf_map_support_seq_show(const struct bpf_map *map)
> {
> - return map->ops->map_seq_show_elem && map->ops->map_check_btf;
> + return map->btf && map->ops->map_seq_show_elem;
> }
>
> extern const struct bpf_map_ops bpf_map_offload_ops;
> diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
> index 2aa55d030..67f0bdf 100644
> --- a/kernel/bpf/arraymap.c
> +++ b/kernel/bpf/arraymap.c
> @@ -358,32 +358,6 @@ static void array_map_seq_show_elem(struct bpf_map *map, void *key,
> rcu_read_unlock();
> }
>
> -static int array_map_check_btf(const struct bpf_map *map, const struct btf *btf,
> - u32 btf_key_id, u32 btf_value_id)
> -{
> - const struct btf_type *key_type, *value_type;
> - u32 key_size, value_size;
> - u32 int_data;
> -
> - key_type = btf_type_id_size(btf, &btf_key_id, &key_size);
> - if (!key_type || BTF_INFO_KIND(key_type->info) != BTF_KIND_INT)
> - return -EINVAL;
> -
> - int_data = *(u32 *)(key_type + 1);
> - /* bpf array can only take a u32 key. This check makes
> - * sure that the btf matches the attr used during map_create.
> - */
> - if (BTF_INT_BITS(int_data) != 32 || key_size != 4 ||
> - BTF_INT_OFFSET(int_data))
> - return -EINVAL;
I think most of these checks are still necessary for array type.
Relaxing BTF array key from BTF_KIND_INT to, for example, BTF_KIND_ENUM
is probably ok, but key being BTF_KIND_PTR or BTF_KIND_ARRAY doesn't makes sense.
For hash maps we probably need hash specific checks too. Otherwise
such sanity checks would need to be in kernel pretty printer and later
in user space too (bpftool and everything that will consume BTF),
since user space won't be able to trust kernel with sane key types.
^ permalink raw reply
* [PATCH] bonding: avoid repeated display of same link status change
From: rama nichanamatlu @ 2018-08-09 21:12 UTC (permalink / raw)
To: netdev
From 9927a1c2a632d9479a80c63b7d9fda59ea8374bc Mon Sep 17 00:00:00 2001
From: Rama Nichanamatlu <rama.nichanamatlu@oracle.com>
Date: Tue, 31 Jul 2018 07:09:52 -0700
Subject: [PATCH] bonding: avoid repeated display of same link status change
When link status change needs to be committed and rtnl lock couldn't be
taken, avoid redisplay of same link status change message.
Signed-off-by: Rama Nichanamatlu <rama.nichanamatlu@oracle.com>
---
drivers/net/bonding/bond_main.c | 6 ++++--
include/net/bonding.h | 1 +
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c
b/drivers/net/bonding/bond_main.c
index 63e3844..3dd1091 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2086,7 +2086,7 @@ static int bond_miimon_inspect(struct bonding *bond)
bond_propose_link_state(slave, BOND_LINK_FAIL);
commit++;
slave->delay = bond->params.downdelay;
- if (slave->delay) {
+ if (slave->delay && !bond->rtnl_needed) {
netdev_info(bond->dev, "link status
down for %sinterface %s, disabling it in %d ms\n",
(BOND_MODE(bond) ==
BOND_MODE_ACTIVEBACKUP) ?
@@ -2126,7 +2126,7 @@ static int bond_miimon_inspect(struct bonding *bond)
commit++;
slave->delay = bond->params.updelay;
- if (slave->delay) {
+ if (slave->delay && !bond->rtnl_needed) {
netdev_info(bond->dev, "link status up
for interface %s, enabling it in %d ms\n",
slave->dev->name,
ignore_updelay ? 0 :
@@ -2300,9 +2300,11 @@ static void bond_mii_monitor(struct work_struct
*work)
if (!rtnl_trylock()) {
delay = 1;
should_notify_peers = false;
+ bond->rtnl_needed = true;
goto re_arm;
}
+ bond->rtnl_needed = false;
bond_for_each_slave(bond, slave, iter) {
bond_commit_link_state(slave,
BOND_SLAVE_NOTIFY_LATER);
}
diff --git a/include/net/bonding.h b/include/net/bonding.h
index 808f1d1..4e76e5d 100644
--- a/include/net/bonding.h
+++ b/include/net/bonding.h
@@ -234,6 +234,7 @@ struct bonding {
struct dentry *debug_dir;
#endif /* CONFIG_DEBUG_FS */
struct rtnl_link_stats64 bond_stats;
+ u8 rtnl_needed;
};
#define bond_slave_get_rcu(dev) \
--
1.7.1
^ permalink raw reply related
* Re: [patch net-next] net: sched: fix block->refcnt decrement
From: David Miller @ 2018-08-09 21:12 UTC (permalink / raw)
To: jiri; +Cc: netdev, jhs, xiyou.wangcong, vladbu, mlxsw
In-Reply-To: <20180808120413.7667-1-jiri@resnulli.us>
From: Jiri Pirko <jiri@resnulli.us>
Date: Wed, 8 Aug 2018 14:04:13 +0200
> From: Jiri Pirko <jiri@mellanox.com>
>
> Currently the refcnt is never decremented in case the value is not 1.
> Fix it by adding decrement in case the refcnt is not 1.
>
> Reported-by: Vlad Buslov <vladbu@mellanox.com>
> Fixes: f71e0ca4db18 ("net: sched: Avoid implicit chain 0 creation")
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Applied.
^ 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