* Re: [PATCH 1/1] mlx5: Fix build when CONFIG_MLX5_EN_RXNFC is disabled
From: Saeed Mahameed @ 2019-06-25 18:00 UTC (permalink / raw)
To: jes.sorensen@gmail.com
Cc: kernel-team@fb.com, jsorensen@fb.com, netdev@vger.kernel.org
In-Reply-To: <20190625152708.23729-2-Jes.Sorensen@gmail.com>
On Tue, 2019-06-25 at 11:27 -0400, Jes Sorensen wrote:
> From: Jes Sorensen <jsorensen@fb.com>
>
> The previous patch broke the build with a static declaration for
> a public function.
>
> Fixes: 8f0916c6dc5c (net/mlx5e: Fix ethtool rxfh commands when
> CONFIG_MLX5_EN_RXNFC is disabled)
> Signed-off-by: Jes Sorensen <jsorensen@fb.com>
> ---
> drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
> b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
> index dd764e0471f2..776040d91bd4 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
> @@ -1905,7 +1905,8 @@ static int mlx5e_flash_device(struct net_device
> *dev,
> /* When CONFIG_MLX5_EN_RXNFC=n we only support ETHTOOL_GRXRINGS
> * otherwise this function will be defined from en_fs_ethtool.c
> */
As the above comment states, when CONFIG_MLX5_EN_RXNFC is disabled,
mlx5e_get_rxnfc is only defined, declared and used in this file, so it
must be static. Otherwise it will be defined in another file which
provides much much more functionality for ethtool flow steering.
can you please provide more information of what went wrong on your
build machine ?
> -static int mlx5e_get_rxnfc(struct net_device *dev, struct
> ethtool_rxnfc *info, u32 *rule_locs)
> +int mlx5e_get_rxnfc(struct net_device *dev, struct ethtool_rxnfc
> *info,
> + u32 *rule_locs)
> {
> struct mlx5e_priv *priv = netdev_priv(dev);
>
^ permalink raw reply
* Re: [PATCH v4 4/7] lib/hexdump.c: Replace ascii bool in hex_dump_to_buffer with flags
From: kbuild test robot @ 2019-06-25 18:00 UTC (permalink / raw)
To: Alastair D'Silva
Cc: kbuild-all, alastair, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
David Airlie, Daniel Vetter, Dan Carpenter, Karsten Keil,
Jassi Brar, Tom Lendacky, David S. Miller, Jose Abreu, Kalle Valo,
Stanislaw Gruszka, Benson Leung, Enric Balletbo i Serra,
James E.J. Bottomley, Martin K. Petersen, Greg Kroah-Hartman,
Alexander Viro, Petr Mladek, Sergey Senozhatsky, Steven Rostedt,
David Laight, Andrew Morton, intel-gfx, dri-devel, linux-kernel,
netdev, ath10k, linux-wireless, linux-scsi, linux-fbdev, devel,
linux-fsdevel
In-Reply-To: <20190625031726.12173-5-alastair@au1.ibm.com>
[-- Attachment #1: Type: text/plain, Size: 4136 bytes --]
Hi Alastair,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v5.2-rc6 next-20190625]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Alastair-D-Silva/Hexdump-Enhancements/20190625-224046
config: sh-allmodconfig (attached as .config)
compiler: sh4-linux-gcc (GCC) 7.4.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.4.0 make.cross ARCH=sh
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
drivers/gpu//drm/tinydrm/core/tinydrm-helpers.c: In function 'tinydrm_dbg_spi_print':
>> drivers/gpu//drm/tinydrm/core/tinydrm-helpers.c:93:2: error: too many arguments to function 'hex_dump_to_buffer'
hex_dump_to_buffer(buf, tr->len, 16,
^~~~~~~~~~~~~~~~~~
In file included from include/linux/kernel.h:15:0,
from include/linux/list.h:9,
from include/linux/kobject.h:19,
from include/linux/device.h:16,
from include/linux/backlight.h:12,
from drivers/gpu//drm/tinydrm/core/tinydrm-helpers.c:6:
include/linux/printk.h:523:19: note: declared here
static inline int hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
^~~~~~~~~~~~~~~~~~
--
sound/soc//sof/xtensa/core.c: In function 'xtensa_stack':
>> sound/soc//sof/xtensa/core.c:125:3: error: too many arguments to function 'hex_dump_to_buffer'
hex_dump_to_buffer(stack + i * 4, 16, 16, 4,
^~~~~~~~~~~~~~~~~~
In file included from include/linux/kernel.h:15:0,
from include/linux/list.h:9,
from include/linux/module.h:9,
from sound/soc//sof/xtensa/core.c:11:
include/linux/printk.h:523:19: note: declared here
static inline int hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
^~~~~~~~~~~~~~~~~~
vim +/hex_dump_to_buffer +93 drivers/gpu//drm/tinydrm/core/tinydrm-helpers.c
9f69eb5c Noralf Trønnes 2017-01-22 85
9f69eb5c Noralf Trønnes 2017-01-22 86 static void
9f69eb5c Noralf Trønnes 2017-01-22 87 tinydrm_dbg_spi_print(struct spi_device *spi, struct spi_transfer *tr,
9f69eb5c Noralf Trønnes 2017-01-22 88 const void *buf, int idx, bool tx)
9f69eb5c Noralf Trønnes 2017-01-22 89 {
9f69eb5c Noralf Trønnes 2017-01-22 90 u32 speed_hz = tr->speed_hz ? tr->speed_hz : spi->max_speed_hz;
9f69eb5c Noralf Trønnes 2017-01-22 91 char linebuf[3 * 32];
9f69eb5c Noralf Trønnes 2017-01-22 92
9f69eb5c Noralf Trønnes 2017-01-22 @93 hex_dump_to_buffer(buf, tr->len, 16,
9f69eb5c Noralf Trønnes 2017-01-22 94 DIV_ROUND_UP(tr->bits_per_word, 8),
9f69eb5c Noralf Trønnes 2017-01-22 95 linebuf, sizeof(linebuf), false);
9f69eb5c Noralf Trønnes 2017-01-22 96
9f69eb5c Noralf Trønnes 2017-01-22 97 printk(KERN_DEBUG
9f69eb5c Noralf Trønnes 2017-01-22 98 " tr(%i): speed=%u%s, bpw=%i, len=%u, %s_buf=[%s%s]\n", idx,
9f69eb5c Noralf Trønnes 2017-01-22 99 speed_hz > 1000000 ? speed_hz / 1000000 : speed_hz / 1000,
9f69eb5c Noralf Trønnes 2017-01-22 100 speed_hz > 1000000 ? "MHz" : "kHz", tr->bits_per_word, tr->len,
9f69eb5c Noralf Trønnes 2017-01-22 101 tx ? "tx" : "rx", linebuf, tr->len > 16 ? " ..." : "");
9f69eb5c Noralf Trønnes 2017-01-22 102 }
9f69eb5c Noralf Trønnes 2017-01-22 103
:::::: The code at line 93 was first introduced by commit
:::::: 9f69eb5c36a644571cca6b2f8dc5f6a7cba04a8b drm/tinydrm: Add helper functions
:::::: TO: Noralf Trønnes <noralf@tronnes.org>
:::::: CC: Noralf Trønnes <noralf@tronnes.org>
---
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: 53198 bytes --]
^ permalink raw reply
* Re: [PATCH 0/1] Fix broken build of mlx5
From: Jes Sorensen @ 2019-06-25 18:01 UTC (permalink / raw)
To: Saeed Mahameed
Cc: kernel-team@fb.com, jsorensen@fb.com, netdev@vger.kernel.org
In-Reply-To: <134e3a684c27fddeeeac111e5b4fac4093473731.camel@mellanox.com>
On 6/25/19 1:54 PM, Saeed Mahameed wrote:
> On Tue, 2019-06-25 at 11:27 -0400, Jes Sorensen wrote:
>> From: Jes Sorensen <jsorensen@fb.com>
>>
>> This fixes an obvious build error that could have been caught by
>> simply building the code before pushing out the patch.
>>
>
> Hi Jes,
>
> Just tested again, as I have tested before submitting the blamed patch,
> and as we test on every single new patch in our build automation.
>
> both combinations CONFIG_MLX5_EN_RXNFC=y/n work on latest net-next,
> what am i missing ?
Linus' tree:
[jes@xpeas linux.git]$ grep mlx5e_get_rxnfc
drivers/net/ethernet/mellanox/mlx5/core/*.c
drivers/net/ethernet/mellanox/mlx5/core/en/*.h
drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c:static int
mlx5e_get_rxnfc(struct net_device *dev, struct ethtool_rxnfc *info, u32
*rule_locs)
drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c: .get_rxnfc
= mlx5e_get_rxnfc,
drivers/net/ethernet/mellanox/mlx5/core/en_fs_ethtool.c:int
mlx5e_get_rxnfc(struct net_device *dev,
drivers/net/ethernet/mellanox/mlx5/core/en/fs.h:int
mlx5e_get_rxnfc(struct net_device *dev,
static vs non static functions, with a prototype that is non static.
Jes
^ permalink raw reply
* Re: [PATCH net] net/sched: flower: fix infinite loop in fl_walk()
From: Cong Wang @ 2019-06-25 18:07 UTC (permalink / raw)
To: Davide Caratti
Cc: Vlad Buslov, David S. Miller, Linux Kernel Network Developers,
Lucas Bates
In-Reply-To: <6650f0da68982ffa5bb71a773c5a3d588bd972c4.camel@redhat.com>
Hello,
On Tue, Jun 25, 2019 at 8:47 AM Davide Caratti <dcaratti@redhat.com> wrote:
> hello Cong,
>
> I tested the above patch, but I still see the infinite loop on kernel
> 5.2.0-0.rc5.git0.1.fc31.i686 .
>
> idr_get_next_ul() returns the entry in the radix tree which is greater or
> equal to '*nextid' (which has the same value as 'id' in the above hunk).
> So, when the radix tree contains one slot with index equal to ULONG_MAX,
> whatever can be the value of 'id', the condition in that if() will always
> be false (and the function will keep returning non-NULL, hence the
> infinite loop).
>
> I also tried this:
>
> if (iter.index == id && id == ULONG_MAX) {
> return NULL;
> }
>
> it fixes the infinite loop, but it clearly breaks the function semantic
> (and anyway, it's not sufficient to fix my test, at least with cls_flower
> it still dumps the entry with id 0xffffffff several times). I'm for
> fixing the callers of idr_get_next_ul(), and in details:
It now becomes more interesting.
On one hand, its callers should not need to worry about details
like overflow. On the other hand, in fact it does exactly what its
callers tell it to do, the problematic part is actually the
incremented id. On 64bit, it is fairly easy, we can just simply
know 'long' is longer than 32bit and leverage this to detect overflow,
but on 32bit this clearly doesn't work.
Let me think about it.
Thanks.
^ permalink raw reply
* Re: general protection fault in sctp_sched_prio_sched
From: Xin Long @ 2019-06-25 18:09 UTC (permalink / raw)
To: Marcelo Ricardo Leitner
Cc: Hillf Danton, syzbot, davem, LKML, linux-sctp, network dev,
Neil Horman, syzkaller-bugs, Vlad Yasevich
In-Reply-To: <20190617144331.GE3500@localhost.localdomain>
On Mon, Jun 17, 2019 at 10:43 PM Marcelo Ricardo Leitner
<marcelo.leitner@gmail.com> wrote:
>
> On Mon, Jun 17, 2019 at 10:49:13AM -0300, Marcelo Ricardo Leitner wrote:
> > Hi,
> >
> > On Sun, Jun 16, 2019 at 11:38:03PM +0800, Hillf Danton wrote:
> > >
> > > Hello Syzbot
> > >
> > > On Sat, 15 Jun 2019 16:36:06 -0700 (PDT) syzbot wrote:
> > > > Hello,
> > > >
> > > > syzbot found the following crash on:
> > > >
> > ...
> > > Check prio_head and bail out if it is not valid.
> > >
> > > Thanks
> > > Hillf
> > > ----->8---
> > > ---
> > > net/sctp/stream_sched_prio.c | 2 ++
> > > 1 file changed, 2 insertions(+)
> > >
> > > diff --git a/net/sctp/stream_sched_prio.c b/net/sctp/stream_sched_prio.c
> > > index 2245083..db25a43 100644
> > > --- a/net/sctp/stream_sched_prio.c
> > > +++ b/net/sctp/stream_sched_prio.c
> > > @@ -135,6 +135,8 @@ static void sctp_sched_prio_sched(struct sctp_stream *stream,
> > > struct sctp_stream_priorities *prio, *prio_head;
> > >
> > > prio_head = soute->prio_head;
> > > + if (!prio_head)
> > > + return;
> > >
> > > /* Nothing to do if already scheduled */
> > > if (!list_empty(&soute->prio_list))
> > > --
> >
> > Thanks but this is not a good fix for this. It will cause the stream
> > to never be scheduled.
> >
> > The problem happens because of the fault injection that happened a bit
> > before the crash, in here:
> >
> > int sctp_stream_init_ext(struct sctp_stream *stream, __u16 sid)
> > {
> > struct sctp_stream_out_ext *soute;
> >
> > soute = kzalloc(sizeof(*soute), GFP_KERNEL);
> > if (!soute)
> > return -ENOMEM;
> > SCTP_SO(stream, sid)->ext = soute; <---- [A]
> >
> > return sctp_sched_init_sid(stream, sid, GFP_KERNEL);
> > ^^^^^^^^^^^^---- [B] failed
> > }
> >
> > This causes the 1st sendmsg to bail out with the error. When the 2nd
> > one gets in, it will:
> >
> > sctp_sendmsg_to_asoc()
> > {
> > ...
> > if (unlikely(!SCTP_SO(&asoc->stream, sinfo->sinfo_stream)->ext)) {
> > ^^^^^--- [C]
> > err = sctp_stream_init_ext(&asoc->stream, sinfo->sinfo_stream);
> > if (err)
> > goto err;
> > }
> >
> > [A] leaves ext initialized, despite the failed in [B]. Then in [C], it
> > will not try to initialize again.
> >
> > We need to either uninitialize ->ext as error handling for [B], or
> > improve the check on [C].
>
> The former one, please. This should be enough (untested):
>
> diff --git a/net/sctp/stream.c b/net/sctp/stream.c
> index 93ed07877337..25946604af85 100644
> --- a/net/sctp/stream.c
> +++ b/net/sctp/stream.c
> @@ -153,13 +153,20 @@ int sctp_stream_init(struct sctp_stream *stream, __u16 outcnt, __u16 incnt,
> int sctp_stream_init_ext(struct sctp_stream *stream, __u16 sid)
> {
> struct sctp_stream_out_ext *soute;
> + int ret;
>
> soute = kzalloc(sizeof(*soute), GFP_KERNEL);
> if (!soute)
> return -ENOMEM;
> SCTP_SO(stream, sid)->ext = soute;
>
> - return sctp_sched_init_sid(stream, sid, GFP_KERNEL);
> + ret = sctp_sched_init_sid(stream, sid, GFP_KERNEL);
> + if (ret) {
> + kfree(SCTP_SO(stream, sid)->ext);
> + SCTP_SO(stream, sid)->ext = NULL;
> + }
> +
> + return ret;
> }
>
> void sctp_stream_free(struct sctp_stream *stream)
>
Tested-by: Xin Long <lucien.xin@gmail.com>
Hi, Marcelo, please feel free to move forward with this patch, :-)
^ permalink raw reply
* Re: [PATCH v2 bpf-next 00/11] BTF-defined BPF map definitions
From: Andrii Nakryiko @ 2019-06-25 18:14 UTC (permalink / raw)
To: Lorenz Bauer
Cc: Daniel Borkmann, Andrii Nakryiko, Alexei Starovoitov, Networking,
bpf, Kernel Team, Jakub Kicinski, Joe Stringer
In-Reply-To: <CAEf4BzYtM-41Y5w0-6kWhaeQUh7it5Zd_OZBj4kvMYou3TwQ+A@mail.gmail.com>
On Fri, Jun 21, 2019 at 10:56 AM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Fri, Jun 21, 2019 at 3:29 AM Lorenz Bauer <lmb@cloudflare.com> wrote:
> >
> > On Fri, 21 Jun 2019 at 05:20, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
> > >
> > > On Thu, Jun 20, 2019 at 7:49 AM Lorenz Bauer <lmb@cloudflare.com> wrote:
> > > >
> > > > On Tue, 18 Jun 2019 at 22:37, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
> > > >
> > > > > > I would just drop the object-scope pinning. We avoided using it and I'm not
> > > > > > aware if anyone else make use. It also has the ugly side-effect that this
> > > > > > relies on AF_ALG which e.g. on some cloud provider shipped kernels is disabled.
> > > > > > The pinning attribute should be part of the standard set of map attributes for
> > > > > > libbpf though as it's generally useful for networking applications.
> > > > >
> > > > > Sounds good. I'll do some more surveying of use cases inside FB to see
> > > > > if anyone needs object-scope pinning, just to be sure we are not
> > > > > short-cutting anyone.
> > > >
> > > > I'm also curious what the use cases for declarative pinning are. From my
> > > > limited POV it doesn't seem that useful? There are a couple of factors:
> > >
> > > Cilium is using it pretty extensively, so there are clearly use cases.
> > > The most straigtforward use case is using a map created and shared by
> > > another BPF program (to communicate, read stats, what have you).
> >
> > I think Cilium is in the quirky position that it has a persistent daemon, but
> > shells out to tc for loading programs. They are probably also the most
> > advanced (open-source) users of BPF out there. If I understood their comments
> > correctly they want to move to using a library for loading their ELF. At that
> > point whether something is possible in a declarative way is less important,
> > because you have the much more powerful APIs at your disposal.
> >
> > Maybe Daniel or someone else from the Cilium team can chime in here?
>
> Yep, curious about their perspective on that.
>
> >
> > > > * Systemd mounts the default location only accessible to root, so I have to
> > > > used my own bpffs mount.
> > > > * Since I don't want to hard code that, I put it in a config file.
> > > > * After loading the ELF we pin maps from the daemon managing the XDP.
> > >
> > > So mounting root would be specified per bpf_object, before maps are
> > > created, so user-land driving application will have an opportunity to
> > > tune everything. Declarative is only the per-map decision of whether
> > > that map should be exposed to outer world (for sharing) or not.
> >
> > So `tc filter add bpf obj foo.elf pin-root /gobbledygook`?
>
> I meant something like:
>
> bpf_object_open_attr attr;
> attr.file = "path/to/my/object.o";
> attr.pin_root_path = "/my/fancy/bpffs/root";
> bpf_object__open_xattr(&attr);
>
> Then tools can adopt they when necessary.
>
> >
> > > Then check tools/testing/selftests/bpf/progs/btf_dump_test_case_syntax.c
> > > for more crazy syntax ;)
> > >
> > > typedef char * (* const (* const fn_ptr_arr2_t[5])())(char * (*)(int));
> >
> > Not on a Friday ;P
> >
> > > > What if this did
> > > >
> > > > __type(value, struct my_value)[1000];
> > > > struct my_value __member(value)[1000]; // alternative
> > > >
> > > > instead, and skipped max_entries?
> > >
> > > I considered that, but decided for now to keep all those attributes
> > > orthogonal for more flexibility and uniformity. This syntax might be
> > > considered a nice "syntax sugar" and can be added in the future, if
> > > necessary.
> >
> > Ack.
> >
> > > > At that point you have to understand that value is a pointer so all of
> > > > our efforts
> > > > are for naught. I suspect there is other weirdness like this, but I need to play
> > > > with it a little bit more.
> > >
> > > Yes, C can let you do crazy stuff, if you wish, but I think that
> > > shouldn't be a blocker for this proposal. I haven't seen any BPF
> > > program doing that, usually you duplicate the type of inner value
> > > inside your function anyway, so there is no point in taking
> > > sizeof(map.value) from BPF program side. From outside, though, all the
> > > types will make sense, as expected.
> >
> > Right, but in my mind that is a bit of a cop out. I like BTF map definitions,
> > and I want them to be as unsurprising as possible, so that they are
> > easy to use and adopt.
>
>
> Right, but there are limit on what you can do with C syntax and it's
> type system. Having fancy extra features like you described (e.g,
> sizeof(map.value), etc) is pretty low on a priority list.
>
> >
> > If a type encodes all the information we need via the array dimension hack,
> > couldn't we make the map variable itself a pointer, and drop the inner pointers?
> >
> > struct my_map_def {
> > int type[BPF_MAP_TYPE_HASH];
> > int value;
> > struct foo key;
>
> This is bad because it potentially uses lots of space. If `struct foo`
> is big, if max_entries is big, even for type, it's still a bunch of
> extra space wasted. That's why we have pointers everywhere, as they
> allow to encode everything with fixed space overhead of 8 bytes for a
> pointer.
>
>
> > ...
> > }
> >
> > struct my_map_def *my_map;
Oh, I missed this point completely, sorry about that.
This has very little advantage over my proposal, in that number
encoding is still cumbersome with array dimensions, so you'd want to
hide it anyway behind macro, probably.
But the main problem with that is when we are going to do prog_array
or map-in-map initialization. This will create potentially huge
anonymous variable to initialize this pointer. See example below:
$ cat test.c
typedef int(*func)(void);
int f1(void) {
return 0;
}
int f2(void) {
return 1;
}
struct my_map_def {
int size[1000];
func arr[1000];
} *map = &(struct my_map_def){
.arr = {
[500] = &f1,
[999] = &f2,
},
};
$ ~/local/llvm/build/bin/clang -g -target bpf -c test.c -o test.o
$ bpftool btf dump file test.o
<snip>
[6] VAR '.compoundliteral' type_id=0, linkage=static
<snip>
[15] DATASEC '.data' size=0 vlen=1
type_id=6 offset=0 size=12000
Note how variable ".compoundliteral" of size 12000 bytes is added
here. Plus the syntax of initialization is cumbersome, and it requires
naming map definition struct just for that &(struct my_map_def) cast.
So I think this doesn't get as much, but makes more advanced use cases
much more cumbersome and prohibitively expensive in terms of storage
size.
> >
> > --
> > Lorenz Bauer | Systems Engineer
> > 6th Floor, County Hall/The Riverside Building, SE1 7PB, UK
> >
> > www.cloudflare.com
^ permalink raw reply
* Re: [RFC PATCH 0/2] enable broadcom tagging for bcm531x5 switches
From: Florian Fainelli @ 2019-06-25 18:17 UTC (permalink / raw)
To: Benedikt Spranger; +Cc: netdev, Sebastian Andrzej Siewior, Kurt Kanzenbach
In-Reply-To: <20190625132026.06a5c543@mitra>
On 6/25/19 4:20 AM, Benedikt Spranger wrote:
> On Sat, 22 Jun 2019 19:24:10 -0700
> Florian Fainelli <f.fainelli@gmail.com> wrote:
>
>> Something like this should take care of that (untested). You might
>> have to explicitly set the IMP port (port 8) in B53_UC_FWD_EN and
>> B53_MC_FWD_EN, though since you turn on management mode, this may not
>> be required. I might have some time tomorrow to test that on a Lamobo
>> R1.
>
> The patch work fine in a manual setup, but not with automagicaly setup
> of $DISTRO. But that is a very different story. Trying to debug whats
> going wrong there.
>
> Thanks for the patch.
>
> The whole setup of the switch seem to be desperate fragile. Any chance
> to get that more robust?
I don't have as much as time as I used to have (both life and work
factors) to contribute to b53, and comments like those really make me to
continue digging my cave and move away from any sort of upstream
development.
This is open source, so if you have the datasheet and feel like you do
better, please go ahead, I certainly won't object and will happily
review patches.
--
Florian
^ permalink raw reply
* [PATCH net-next 1/1] tc-testing: add ingress qdisc tests
From: Roman Mashak @ 2019-06-25 18:18 UTC (permalink / raw)
To: davem; +Cc: netdev, kernel, jhs, xiyou.wangcong, jiri, Roman Mashak
Signed-off-by: Roman Mashak <mrv@mojatatu.com>
---
.../tc-testing/tc-tests/qdiscs/ingress.json | 102 +++++++++++++++++++++
1 file changed, 102 insertions(+)
create mode 100644 tools/testing/selftests/tc-testing/tc-tests/qdiscs/ingress.json
diff --git a/tools/testing/selftests/tc-testing/tc-tests/qdiscs/ingress.json b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/ingress.json
new file mode 100644
index 000000000000..f518c55f468b
--- /dev/null
+++ b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/ingress.json
@@ -0,0 +1,102 @@
+[
+ {
+ "id": "9872",
+ "name": "Add ingress qdisc",
+ "category": [
+ "qdisc",
+ "ingress"
+ ],
+ "setup": [
+ "$IP link add dev $DEV1 type dummy || /bin/true"
+ ],
+ "cmdUnderTest": "$TC qdisc add dev $DEV1 ingress",
+ "expExitCode": "0",
+ "verifyCmd": "$TC qdisc show dev $DEV1",
+ "matchPattern": "qdisc ingress ffff:",
+ "matchCount": "1",
+ "teardown": [
+ "$TC qdisc del dev $DEV1 ingress",
+ "$IP link del dev $DEV1 type dummy"
+ ]
+ },
+ {
+ "id": "5c5e",
+ "name": "Add ingress qdisc with unsupported argument",
+ "category": [
+ "qdisc",
+ "ingress"
+ ],
+ "setup": [
+ "$IP link add dev $DEV1 type dummy || /bin/true"
+ ],
+ "cmdUnderTest": "$TC qdisc add dev $DEV1 ingress foorbar",
+ "expExitCode": "1",
+ "verifyCmd": "$TC qdisc show dev $DEV1",
+ "matchPattern": "qdisc ingress ffff:",
+ "matchCount": "0",
+ "teardown": [
+ "$IP link del dev $DEV1 type dummy"
+ ]
+ },
+ {
+ "id": "74f6",
+ "name": "Add duplicate ingress qdisc",
+ "category": [
+ "qdisc",
+ "ingress"
+ ],
+ "setup": [
+ "$IP link add dev $DEV1 type dummy || /bin/true",
+ "$TC qdisc add dev $DEV1 ingress"
+ ],
+ "cmdUnderTest": "$TC qdisc add dev $DEV1 ingress",
+ "expExitCode": "2",
+ "verifyCmd": "$TC qdisc show dev $DEV1",
+ "matchPattern": "qdisc ingress ffff:",
+ "matchCount": "1",
+ "teardown": [
+ "$TC qdisc del dev $DEV1 ingress",
+ "$IP link del dev $DEV1 type dummy"
+ ]
+ },
+ {
+ "id": "f769",
+ "name": "Delete nonexistent ingress qdisc",
+ "category": [
+ "qdisc",
+ "ingress"
+ ],
+ "setup": [
+ "$IP link add dev $DEV1 type dummy || /bin/true"
+ ],
+ "cmdUnderTest": "$TC qdisc del dev $DEV1 ingress",
+ "expExitCode": "2",
+ "verifyCmd": "$TC qdisc show dev $DEV1",
+ "matchPattern": "qdisc ingress ffff:",
+ "matchCount": "0",
+ "teardown": [
+ "$IP link del dev $DEV1 type dummy"
+ ]
+ },
+ {
+ "id": "3b88",
+ "name": "Delete ingress qdisc twice",
+ "category": [
+ "qdisc",
+ "ingress"
+ ],
+ "setup": [
+ "$IP link add dev $DEV1 type dummy || /bin/true",
+ "$TC qdisc add dev $DEV1 ingress",
+ "$TC qdisc del dev $DEV1 ingress"
+ ],
+ "cmdUnderTest": "$TC qdisc del dev $DEV1 ingress",
+ "expExitCode": "2",
+ "verifyCmd": "$TC qdisc show dev $DEV1",
+ "matchPattern": "qdisc ingress ffff:",
+ "matchCount": "0",
+ "teardown": [
+ "$IP link del dev $DEV1 type dummy"
+ ]
+ }
+]
--
2.7.4
^ permalink raw reply related
* Re: Removing skb_orphan() from ip_rcv_core()
From: Joe Stringer @ 2019-06-25 18:20 UTC (permalink / raw)
To: Eric Dumazet
Cc: Joe Stringer, Florian Westphal, netdev, john fastabend,
Daniel Borkmann, Lorenz Bauer, Jakub Sitnicki, Paolo Abeni
In-Reply-To: <b6baadcb-29af-82f1-bebe-56d5f45b12e6@gmail.com>
On Mon, Jun 24, 2019 at 11:37 PM Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On 6/24/19 8:17 PM, Joe Stringer wrote:
> > On Fri, Jun 21, 2019 at 1:59 PM Florian Westphal <fw@strlen.de> wrote:
> >> Joe Stringer <joe@wand.net.nz> wrote:
> >>> However, if I drop these lines then I end up causing sockets to
> >>> release references too many times. Seems like if we don't orphan the
> >>> skb here, then later logic assumes that we have one more reference
> >>> than we actually have, and decrements the count when it shouldn't
> >>> (perhaps the skb_steal_sock() call in __inet_lookup_skb() which seems
> >>> to assume we always have a reference to the socket?)
> >>
> >> We might be calling the wrong destructor (i.e., the one set by tcp
> >> receive instead of the one set at tx time)?
> >
> > Hmm, interesting thought. Sure enough, with a bit of bpftrace
> > debugging we find it's tcp_wfree():
> >
> > $ cat ip_rcv.bt
> > #include <linux/skbuff.h>
> >
> > kprobe:ip_rcv {
> > $sk = ((struct sk_buff *)arg0)->sk;
> > $des = ((struct sk_buff *)arg0)->destructor;
> > if ($sk) {
> > if ($des) {
> > printf("received %s on %s with sk destructor %s
> > set\n", str(arg0), str(arg1), ksym($des));
> > @ip4_stacks[kstack] = count();
> > }
> > }
> > }
> > $ sudo bpftrace ip_rcv.bt
> > Attaching 1 probe...
> > received on eth0 with sk destructor tcp_wfree set
> > ^C
> >
> > @ip4_stacks[
> > ip_rcv+1
> > __netif_receive_skb+24
> > process_backlog+179
> > net_rx_action+304
> > __do_softirq+220
> > do_softirq_own_stack+42
> > do_softirq.part.17+70
> > __local_bh_enable_ip+101
> > ip_finish_output2+421
> > __ip_finish_output+187
> > ip_finish_output+44
> > ip_output+109
> > ip_local_out+59
> > __ip_queue_xmit+368
> > ip_queue_xmit+16
> > __tcp_transmit_skb+1303
> > tcp_connect+2758
> > tcp_v4_connect+1135
> > __inet_stream_connect+214
> > inet_stream_connect+59
> > __sys_connect+237
> > __x64_sys_connect+26
> > do_syscall_64+90
> > entry_SYSCALL_64_after_hwframe+68
> > ]: 1
> >
> > Is there a solution here where we call the destructor if it's not
> > sock_efree()? When the socket is later stolen, it will only return the
> > reference via a call to sock_put(), so presumably at that point in the
> > stack we already assume that the skb->destructor is not one of these
> > other destructors (otherwise we wouldn't release the resources
> > correctly).
> >
>
> What was the driver here ? In any case, the following patch should help.
>
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index eeacebd7debbe6a55daedb92f00afd48051ebaf8..5075b4b267af7057f69fcb935226fce097a920e2 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -3699,6 +3699,7 @@ static __always_inline int ____dev_forward_skb(struct net_device *dev,
> return NET_RX_DROP;
> }
>
> + skb_orphan(skb);
> skb_scrub_packet(skb, true);
> skb->priority = 0;
> return 0;
Looks like it was bridge in the end, found by attaching a similar
bpftrace program to __dev_forward_sk(). Interestingly enough, the
device attached to the skb reported its name as "eth0" despite not
having such a named link or named bridge that I could find anywhere
via "ip link" / "brctl show"..
__dev_forward_skb+1
dev_hard_start_xmit+151
__dev_queue_xmit+1928
dev_queue_xmit+16
br_dev_queue_push_xmit+123
br_forward_finish+69
__br_forward+327
br_forward+204
br_dev_xmit+598
dev_hard_start_xmit+151
__dev_queue_xmit+1928
dev_queue_xmit+16
neigh_resolve_output+339
ip_finish_output2+402
__ip_finish_output+187
ip_finish_output+44
ip_output+109
ip_local_out+59
__ip_queue_xmit+368
ip_queue_xmit+16
__tcp_transmit_skb+1303
tcp_connect+2758
tcp_v4_connect+1135
__inet_stream_connect+214
inet_stream_connect+59
__sys_connect+237
__x64_sys_connect+26
do_syscall_64+90
entry_SYSCALL_64_after_hwframe+68
So I guess something like this could be another alternative:
diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c
index 82225b8b54f5..c2de2bb35080 100644
--- a/net/bridge/br_forward.c
+++ b/net/bridge/br_forward.c
@@ -65,6 +65,7 @@ EXPORT_SYMBOL_GPL(br_dev_queue_push_xmit);
int br_forward_finish(struct net *net, struct sock *sk, struct sk_buff *skb)
{
+ skb_orphan(skb);
skb->tstamp = 0;
return NF_HOOK(NFPROTO_BRIDGE, NF_BR_POST_ROUTING,
net, sk, skb, NULL, skb->dev,
^ permalink raw reply related
* [PATCH bpf-next 0/4] sys_bpf() access control via /dev/bpf
From: Song Liu @ 2019-06-25 18:22 UTC (permalink / raw)
To: netdev, bpf; +Cc: ast, daniel, kernel-team, Song Liu
Currently, most access to sys_bpf() is limited to root. However, there are
use cases that would benefit from non-privileged use of sys_bpf(), e.g.
systemd.
This set introduces a new model to control the access to sys_bpf(). A
special device, /dev/bpf, is introduced to manage access to sys_bpf().
Users with access to open /dev/bpf will be able to access most of
sys_bpf() features. The use can get access to sys_bpf() by opening /dev/bpf
and use ioctl to get/put permission.
The permission to access sys_bpf() is marked by bit TASK_BPF_FLAG_PERMITTED
in task_struct. During fork(), child will not inherit this bit.
libbpf APIs libbpf_[get|put]_bpf_permission() are added to help get and
put the permission. bpftool is updated to use these APIs.
Song Liu (4):
bpf: unprivileged BPF access via /dev/bpf
bpf: sync tools/include/uapi/linux/bpf.h
libbpf: add libbpf_[get|put]_bpf_permission()
bpftool: use libbpf_[get|put]_bpf_permission()
Documentation/ioctl/ioctl-number.txt | 1 +
include/linux/bpf.h | 12 +++++
include/linux/sched.h | 8 ++++
include/uapi/linux/bpf.h | 5 ++
kernel/bpf/arraymap.c | 2 +-
kernel/bpf/cgroup.c | 2 +-
kernel/bpf/core.c | 4 +-
kernel/bpf/cpumap.c | 2 +-
kernel/bpf/devmap.c | 2 +-
kernel/bpf/hashtab.c | 4 +-
kernel/bpf/lpm_trie.c | 2 +-
kernel/bpf/offload.c | 2 +-
kernel/bpf/queue_stack_maps.c | 2 +-
kernel/bpf/reuseport_array.c | 2 +-
kernel/bpf/stackmap.c | 2 +-
kernel/bpf/syscall.c | 72 +++++++++++++++++++++-------
kernel/bpf/verifier.c | 2 +-
kernel/bpf/xskmap.c | 2 +-
kernel/fork.c | 4 ++
net/core/filter.c | 6 +--
tools/bpf/bpftool/feature.c | 2 +-
tools/bpf/bpftool/main.c | 5 ++
tools/include/uapi/linux/bpf.h | 5 ++
tools/lib/bpf/libbpf.c | 54 +++++++++++++++++++++
tools/lib/bpf/libbpf.h | 7 +++
tools/lib/bpf/libbpf.map | 2 +
26 files changed, 178 insertions(+), 35 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH bpf-next 2/4] bpf: sync tools/include/uapi/linux/bpf.h
From: Song Liu @ 2019-06-25 18:23 UTC (permalink / raw)
To: netdev, bpf; +Cc: ast, daniel, kernel-team, Song Liu
In-Reply-To: <20190625182303.874270-1-songliubraving@fb.com>
Sync changes for bpf_dev_ioctl.
Signed-off-by: Song Liu <songliubraving@fb.com>
---
tools/include/uapi/linux/bpf.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index b077507efa3f..ec3ae452cfd7 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -3541,4 +3541,9 @@ struct bpf_sysctl {
*/
};
+#define BPF_IOCTL 0xBF
+
+#define BPF_DEV_IOCTL_GET_PERM _IO(BPF_IOCTL, 0x01)
+#define BPF_DEV_IOCTL_PUT_PERM _IO(BPF_IOCTL, 0x02)
+
#endif /* _UAPI__LINUX_BPF_H__ */
--
2.17.1
^ permalink raw reply related
* [PATCH bpf-next 3/4] libbpf: add libbpf_[get|put]_bpf_permission()
From: Song Liu @ 2019-06-25 18:23 UTC (permalink / raw)
To: netdev, bpf; +Cc: ast, daniel, kernel-team, Song Liu
In-Reply-To: <20190625182303.874270-1-songliubraving@fb.com>
This patch adds two more API to libbpf: libbpf_get_bpf_permission() and
libbpf_put_bpf_permission().
For root, these two APIs are no-op.
Signed-off-by: Song Liu <songliubraving@fb.com>
---
tools/lib/bpf/libbpf.c | 54 ++++++++++++++++++++++++++++++++++++++++
tools/lib/bpf/libbpf.h | 7 ++++++
tools/lib/bpf/libbpf.map | 2 ++
3 files changed, 63 insertions(+)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 68f45a96769f..cf2d68268bde 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -35,6 +35,7 @@
#include <sys/stat.h>
#include <sys/types.h>
#include <sys/vfs.h>
+#include <sys/ioctl.h>
#include <tools/libc_compat.h>
#include <libelf.h>
#include <gelf.h>
@@ -4286,3 +4287,56 @@ int libbpf_num_possible_cpus(void)
}
return cpus;
}
+
+LIBBPF_API bool libbpf_get_bpf_permission(void)
+{
+ char *cp, errmsg[STRERR_BUFSIZE];
+ int fd, ret;
+
+ if (geteuid() == 0)
+ return true;
+
+ fd = open(LIBBPF_DEV_BPF, O_RDONLY);
+ if (fd < 0) {
+ cp = libbpf_strerror_r(errno, errmsg, sizeof(errmsg));
+ pr_warning("failed to open %s: %s\n", LIBBPF_DEV_BPF, cp);
+ return false;
+ }
+
+ ret = ioctl(fd, BPF_DEV_IOCTL_GET_PERM);
+
+ if (ret) {
+ cp = libbpf_strerror_r(errno, errmsg, sizeof(errmsg));
+ pr_warning("failed to get BPF permission: %s\n", cp);
+ close(fd);
+ return false;
+ }
+ close(fd);
+ pr_debug("got BPF permission for non-privileged user\n");
+ return true;
+}
+
+LIBBPF_API void libbpf_put_bpf_permission(void)
+{
+ char *cp, errmsg[STRERR_BUFSIZE];
+ int fd, ret;
+
+ if (geteuid() == 0)
+ return;
+
+ fd = open(LIBBPF_DEV_BPF, O_RDONLY);
+ if (fd < 0) {
+ cp = libbpf_strerror_r(errno, errmsg, sizeof(errmsg));
+ pr_warning("failed to open %s: %s\n", LIBBPF_DEV_BPF, cp);
+ return;
+ }
+
+ ret = ioctl(fd, BPF_DEV_IOCTL_PUT_PERM);
+ if (ret) {
+ cp = libbpf_strerror_r(errno, errmsg, sizeof(errmsg));
+ pr_warning("failed to release BPF permission: %s\n", cp);
+ close(fd);
+ }
+ close(fd);
+ pr_debug("released BPF permission for non-privileged user\n");
+}
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index d639f47e3110..22052c55a96c 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -470,6 +470,13 @@ bpf_program__bpil_offs_to_addr(struct bpf_prog_info_linear *info_linear);
*/
LIBBPF_API int libbpf_num_possible_cpus(void);
+#define LIBBPF_DEV_BPF "/dev/bpf"
+
+/* (For non-root user) get permission to access bpf() syscall */
+LIBBPF_API bool libbpf_get_bpf_permission(void);
+/* (For non-root user) put permission to access bpf() syscall */
+LIBBPF_API void libbpf_put_bpf_permission(void);
+
#ifdef __cplusplus
} /* extern "C" */
#endif
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index 2c6d835620d2..93a2c4175fdd 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -173,4 +173,6 @@ LIBBPF_0.0.4 {
btf__parse_elf;
bpf_object__load_xattr;
libbpf_num_possible_cpus;
+ libbpf_get_bpf_permission;
+ libbpf_put_bpf_permission;
} LIBBPF_0.0.3;
--
2.17.1
^ permalink raw reply related
* [PATCH bpf-next 1/4] bpf: unprivileged BPF access via /dev/bpf
From: Song Liu @ 2019-06-25 18:23 UTC (permalink / raw)
To: netdev, bpf; +Cc: ast, daniel, kernel-team, Song Liu
In-Reply-To: <20190625182303.874270-1-songliubraving@fb.com>
This patch introduce unprivileged BPF access. The access control is
achieved via device /dev/bpf. Users with access to /dev/bpf are able
to access BPF syscall.
Two ioctl command are added to /dev/bpf:
The first two commands get/put permission to access sys_bpf. This
permission is noted by setting bit TASK_BPF_FLAG_PERMITTED of
current->bpf_flags. This permission cannot be inherited via fork().
Helper function bpf_capable() is added to check whether the task has got
permission via /dev/bpf.
Signed-off-by: Song Liu <songliubraving@fb.com>
---
Documentation/ioctl/ioctl-number.txt | 1 +
include/linux/bpf.h | 12 +++++
include/linux/sched.h | 8 ++++
include/uapi/linux/bpf.h | 5 ++
kernel/bpf/arraymap.c | 2 +-
kernel/bpf/cgroup.c | 2 +-
kernel/bpf/core.c | 4 +-
kernel/bpf/cpumap.c | 2 +-
kernel/bpf/devmap.c | 2 +-
kernel/bpf/hashtab.c | 4 +-
kernel/bpf/lpm_trie.c | 2 +-
kernel/bpf/offload.c | 2 +-
kernel/bpf/queue_stack_maps.c | 2 +-
kernel/bpf/reuseport_array.c | 2 +-
kernel/bpf/stackmap.c | 2 +-
kernel/bpf/syscall.c | 72 +++++++++++++++++++++-------
kernel/bpf/verifier.c | 2 +-
kernel/bpf/xskmap.c | 2 +-
kernel/fork.c | 4 ++
net/core/filter.c | 6 +--
20 files changed, 104 insertions(+), 34 deletions(-)
diff --git a/Documentation/ioctl/ioctl-number.txt b/Documentation/ioctl/ioctl-number.txt
index c9558146ac58..19998b99d603 100644
--- a/Documentation/ioctl/ioctl-number.txt
+++ b/Documentation/ioctl/ioctl-number.txt
@@ -327,6 +327,7 @@ Code Seq#(hex) Include File Comments
0xB4 00-0F linux/gpio.h <mailto:linux-gpio@vger.kernel.org>
0xB5 00-0F uapi/linux/rpmsg.h <mailto:linux-remoteproc@vger.kernel.org>
0xB6 all linux/fpga-dfl.h
+0xBP 01-02 uapi/linux/bpf.h <mailto:bpf@vger.kernel.org>
0xC0 00-0F linux/usb/iowarrior.h
0xCA 00-0F uapi/misc/cxl.h
0xCA 10-2F uapi/misc/ocxl.h
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index a62e7889b0b6..dbba7870f6df 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -14,6 +14,10 @@
#include <linux/numa.h>
#include <linux/wait.h>
#include <linux/u64_stats_sync.h>
+#include <linux/sched.h>
+#include <linux/capability.h>
+
+#include <asm/current.h>
struct bpf_verifier_env;
struct perf_event;
@@ -742,6 +746,12 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
int bpf_prog_test_run_flow_dissector(struct bpf_prog *prog,
const union bpf_attr *kattr,
union bpf_attr __user *uattr);
+
+static inline bool bpf_capable(int cap)
+{
+ return test_bit(TASK_BPF_FLAG_PERMITTED, ¤t->bpf_flags) ||
+ capable(cap);
+}
#else /* !CONFIG_BPF_SYSCALL */
static inline struct bpf_prog *bpf_prog_get(u32 ufd)
{
@@ -874,6 +884,8 @@ static inline int bpf_prog_test_run_flow_dissector(struct bpf_prog *prog,
{
return -ENOTSUPP;
}
+
+#define bpf_capable(cap) capable((cap))
#endif /* CONFIG_BPF_SYSCALL */
static inline struct bpf_prog *bpf_prog_get_type(u32 ufd,
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 11837410690f..ddd33d4476c5 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1200,6 +1200,10 @@ struct task_struct {
unsigned long prev_lowest_stack;
#endif
+#ifdef CONFIG_BPF_SYSCALL
+ unsigned long bpf_flags;
+#endif
+
/*
* New fields for task_struct should be added above here, so that
* they are included in the randomized portion of task_struct.
@@ -1772,6 +1776,10 @@ static inline void set_task_cpu(struct task_struct *p, unsigned int cpu)
#endif /* CONFIG_SMP */
+enum task_struct_bpf_flags {
+ TASK_BPF_FLAG_PERMITTED,
+};
+
/*
* In order to reduce various lock holder preemption latencies provide an
* interface to see if a vCPU is currently running or not.
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index b077507efa3f..ec3ae452cfd7 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -3541,4 +3541,9 @@ struct bpf_sysctl {
*/
};
+#define BPF_IOCTL 0xBF
+
+#define BPF_DEV_IOCTL_GET_PERM _IO(BPF_IOCTL, 0x01)
+#define BPF_DEV_IOCTL_PUT_PERM _IO(BPF_IOCTL, 0x02)
+
#endif /* _UAPI__LINUX_BPF_H__ */
diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
index 1c65ce0098a9..9ae668fa9185 100644
--- a/kernel/bpf/arraymap.c
+++ b/kernel/bpf/arraymap.c
@@ -73,7 +73,7 @@ static struct bpf_map *array_map_alloc(union bpf_attr *attr)
bool percpu = attr->map_type == BPF_MAP_TYPE_PERCPU_ARRAY;
int ret, numa_node = bpf_map_attr_numa_node(attr);
u32 elem_size, index_mask, max_entries;
- bool unpriv = !capable(CAP_SYS_ADMIN);
+ bool unpriv = !bpf_capable(CAP_SYS_ADMIN);
u64 cost, array_size, mask64;
struct bpf_map_memory mem;
struct bpf_array *array;
diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index c225c42e114a..fd9bea70f8f3 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -777,7 +777,7 @@ cgroup_base_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
case BPF_FUNC_get_current_cgroup_id:
return &bpf_get_current_cgroup_id_proto;
case BPF_FUNC_trace_printk:
- if (capable(CAP_SYS_ADMIN))
+ if (bpf_capable(CAP_SYS_ADMIN))
return bpf_get_trace_printk_proto();
/* fall through */
default:
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index ad3be85f1411..25c1e3c59699 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -646,7 +646,7 @@ static bool bpf_prog_kallsyms_verify_off(const struct bpf_prog *fp)
void bpf_prog_kallsyms_add(struct bpf_prog *fp)
{
if (!bpf_prog_kallsyms_candidate(fp) ||
- !capable(CAP_SYS_ADMIN))
+ !bpf_capable(CAP_SYS_ADMIN))
return;
spin_lock_bh(&bpf_lock);
@@ -768,7 +768,7 @@ static int bpf_jit_charge_modmem(u32 pages)
{
if (atomic_long_add_return(pages, &bpf_jit_current) >
(bpf_jit_limit >> PAGE_SHIFT)) {
- if (!capable(CAP_SYS_ADMIN)) {
+ if (!bpf_capable(CAP_SYS_ADMIN)) {
atomic_long_sub(pages, &bpf_jit_current);
return -EPERM;
}
diff --git a/kernel/bpf/cpumap.c b/kernel/bpf/cpumap.c
index 8dff08768087..4c6054626b4f 100644
--- a/kernel/bpf/cpumap.c
+++ b/kernel/bpf/cpumap.c
@@ -83,7 +83,7 @@ static struct bpf_map *cpu_map_alloc(union bpf_attr *attr)
u64 cost;
int ret;
- if (!capable(CAP_SYS_ADMIN))
+ if (!bpf_capable(CAP_SYS_ADMIN))
return ERR_PTR(-EPERM);
/* check sanity of attributes */
diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
index 40e86a7e0ef0..b7c3785be289 100644
--- a/kernel/bpf/devmap.c
+++ b/kernel/bpf/devmap.c
@@ -83,7 +83,7 @@ static struct bpf_map *dev_map_alloc(union bpf_attr *attr)
u64 cost;
int err;
- if (!capable(CAP_NET_ADMIN))
+ if (!bpf_capable(CAP_NET_ADMIN))
return ERR_PTR(-EPERM);
/* check sanity of attributes */
diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
index 22066a62c8c9..461a75c311a4 100644
--- a/kernel/bpf/hashtab.c
+++ b/kernel/bpf/hashtab.c
@@ -244,13 +244,13 @@ static int htab_map_alloc_check(union bpf_attr *attr)
BUILD_BUG_ON(offsetof(struct htab_elem, fnode.next) !=
offsetof(struct htab_elem, hash_node.pprev));
- if (lru && !capable(CAP_SYS_ADMIN))
+ if (lru && !bpf_capable(CAP_SYS_ADMIN))
/* LRU implementation is much complicated than other
* maps. Hence, limit to CAP_SYS_ADMIN for now.
*/
return -EPERM;
- if (zero_seed && !capable(CAP_SYS_ADMIN))
+ if (zero_seed && !bpf_capable(CAP_SYS_ADMIN))
/* Guard against local DoS, and discourage production use. */
return -EPERM;
diff --git a/kernel/bpf/lpm_trie.c b/kernel/bpf/lpm_trie.c
index 56e6c75d354d..571962022fdf 100644
--- a/kernel/bpf/lpm_trie.c
+++ b/kernel/bpf/lpm_trie.c
@@ -543,7 +543,7 @@ static struct bpf_map *trie_alloc(union bpf_attr *attr)
u64 cost = sizeof(*trie), cost_per_node;
int ret;
- if (!capable(CAP_SYS_ADMIN))
+ if (!bpf_capable(CAP_SYS_ADMIN))
return ERR_PTR(-EPERM);
/* check sanity of attributes */
diff --git a/kernel/bpf/offload.c b/kernel/bpf/offload.c
index ba635209ae9a..d3e5378c5a15 100644
--- a/kernel/bpf/offload.c
+++ b/kernel/bpf/offload.c
@@ -366,7 +366,7 @@ struct bpf_map *bpf_map_offload_map_alloc(union bpf_attr *attr)
struct bpf_offloaded_map *offmap;
int err;
- if (!capable(CAP_SYS_ADMIN))
+ if (!bpf_capable(CAP_SYS_ADMIN))
return ERR_PTR(-EPERM);
if (attr->map_type != BPF_MAP_TYPE_ARRAY &&
attr->map_type != BPF_MAP_TYPE_HASH)
diff --git a/kernel/bpf/queue_stack_maps.c b/kernel/bpf/queue_stack_maps.c
index f697647ceb54..01d848f1a783 100644
--- a/kernel/bpf/queue_stack_maps.c
+++ b/kernel/bpf/queue_stack_maps.c
@@ -45,7 +45,7 @@ static bool queue_stack_map_is_full(struct bpf_queue_stack *qs)
/* Called from syscall */
static int queue_stack_map_alloc_check(union bpf_attr *attr)
{
- if (!capable(CAP_SYS_ADMIN))
+ if (!bpf_capable(CAP_SYS_ADMIN))
return -EPERM;
/* check sanity of attributes */
diff --git a/kernel/bpf/reuseport_array.c b/kernel/bpf/reuseport_array.c
index 50c083ba978c..840f38a58c7d 100644
--- a/kernel/bpf/reuseport_array.c
+++ b/kernel/bpf/reuseport_array.c
@@ -154,7 +154,7 @@ static struct bpf_map *reuseport_array_alloc(union bpf_attr *attr)
struct bpf_map_memory mem;
u64 array_size;
- if (!capable(CAP_SYS_ADMIN))
+ if (!bpf_capable(CAP_SYS_ADMIN))
return ERR_PTR(-EPERM);
array_size = sizeof(*array);
diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
index 052580c33d26..1eab27b0bc17 100644
--- a/kernel/bpf/stackmap.c
+++ b/kernel/bpf/stackmap.c
@@ -90,7 +90,7 @@ static struct bpf_map *stack_map_alloc(union bpf_attr *attr)
u64 cost, n_buckets;
int err;
- if (!capable(CAP_SYS_ADMIN))
+ if (!bpf_capable(CAP_SYS_ADMIN))
return ERR_PTR(-EPERM);
if (attr->map_flags & ~STACK_CREATE_FLAG_MASK)
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 7713cf39795a..d80b04b6a5fa 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -23,6 +23,8 @@
#include <linux/timekeeping.h>
#include <linux/ctype.h>
#include <linux/nospec.h>
+#include <linux/miscdevice.h>
+#include <linux/resource.h>
#define IS_FD_ARRAY(map) ((map)->map_type == BPF_MAP_TYPE_PROG_ARRAY || \
(map)->map_type == BPF_MAP_TYPE_PERF_EVENT_ARRAY || \
@@ -1166,7 +1168,7 @@ static int map_freeze(const union bpf_attr *attr)
err = -EBUSY;
goto err_put;
}
- if (!capable(CAP_SYS_ADMIN)) {
+ if (!bpf_capable(CAP_SYS_ADMIN)) {
err = -EPERM;
goto err_put;
}
@@ -1616,7 +1618,7 @@ static int bpf_prog_load(union bpf_attr *attr, union bpf_attr __user *uattr)
if (!IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) &&
(attr->prog_flags & BPF_F_ANY_ALIGNMENT) &&
- !capable(CAP_SYS_ADMIN))
+ !bpf_capable(CAP_SYS_ADMIN))
return -EPERM;
/* copy eBPF program license from user space */
@@ -1629,11 +1631,12 @@ static int bpf_prog_load(union bpf_attr *attr, union bpf_attr __user *uattr)
is_gpl = license_is_gpl_compatible(license);
if (attr->insn_cnt == 0 ||
- attr->insn_cnt > (capable(CAP_SYS_ADMIN) ? BPF_COMPLEXITY_LIMIT_INSNS : BPF_MAXINSNS))
+ attr->insn_cnt > (bpf_capable(CAP_SYS_ADMIN) ?
+ BPF_COMPLEXITY_LIMIT_INSNS : BPF_MAXINSNS))
return -E2BIG;
if (type != BPF_PROG_TYPE_SOCKET_FILTER &&
type != BPF_PROG_TYPE_CGROUP_SKB &&
- !capable(CAP_SYS_ADMIN))
+ !bpf_capable(CAP_SYS_ADMIN))
return -EPERM;
bpf_prog_load_fixup_attach_type(attr);
@@ -1861,7 +1864,7 @@ static int bpf_prog_attach(const union bpf_attr *attr)
struct bpf_prog *prog;
int ret;
- if (!capable(CAP_NET_ADMIN))
+ if (!bpf_capable(CAP_NET_ADMIN))
return -EPERM;
if (CHECK_ATTR(BPF_PROG_ATTACH))
@@ -1951,7 +1954,7 @@ static int bpf_prog_detach(const union bpf_attr *attr)
{
enum bpf_prog_type ptype;
- if (!capable(CAP_NET_ADMIN))
+ if (!bpf_capable(CAP_NET_ADMIN))
return -EPERM;
if (CHECK_ATTR(BPF_PROG_DETACH))
@@ -2007,7 +2010,7 @@ static int bpf_prog_detach(const union bpf_attr *attr)
static int bpf_prog_query(const union bpf_attr *attr,
union bpf_attr __user *uattr)
{
- if (!capable(CAP_NET_ADMIN))
+ if (!bpf_capable(CAP_NET_ADMIN))
return -EPERM;
if (CHECK_ATTR(BPF_PROG_QUERY))
return -EINVAL;
@@ -2051,7 +2054,7 @@ static int bpf_prog_test_run(const union bpf_attr *attr,
struct bpf_prog *prog;
int ret = -ENOTSUPP;
- if (!capable(CAP_SYS_ADMIN))
+ if (!bpf_capable(CAP_SYS_ADMIN))
return -EPERM;
if (CHECK_ATTR(BPF_PROG_TEST_RUN))
return -EINVAL;
@@ -2088,7 +2091,7 @@ static int bpf_obj_get_next_id(const union bpf_attr *attr,
if (CHECK_ATTR(BPF_OBJ_GET_NEXT_ID) || next_id >= INT_MAX)
return -EINVAL;
- if (!capable(CAP_SYS_ADMIN))
+ if (!bpf_capable(CAP_SYS_ADMIN))
return -EPERM;
next_id++;
@@ -2114,7 +2117,7 @@ static int bpf_prog_get_fd_by_id(const union bpf_attr *attr)
if (CHECK_ATTR(BPF_PROG_GET_FD_BY_ID))
return -EINVAL;
- if (!capable(CAP_SYS_ADMIN))
+ if (!bpf_capable(CAP_SYS_ADMIN))
return -EPERM;
spin_lock_bh(&prog_idr_lock);
@@ -2148,7 +2151,7 @@ static int bpf_map_get_fd_by_id(const union bpf_attr *attr)
attr->open_flags & ~BPF_OBJ_FLAG_MASK)
return -EINVAL;
- if (!capable(CAP_SYS_ADMIN))
+ if (!bpf_capable(CAP_SYS_ADMIN))
return -EPERM;
f_flags = bpf_get_file_flag(attr->open_flags);
@@ -2323,7 +2326,7 @@ static int bpf_prog_get_info_by_fd(struct bpf_prog *prog,
info.run_time_ns = stats.nsecs;
info.run_cnt = stats.cnt;
- if (!capable(CAP_SYS_ADMIN)) {
+ if (!bpf_capable(CAP_SYS_ADMIN)) {
info.jited_prog_len = 0;
info.xlated_prog_len = 0;
info.nr_jited_ksyms = 0;
@@ -2641,7 +2644,7 @@ static int bpf_btf_load(const union bpf_attr *attr)
if (CHECK_ATTR(BPF_BTF_LOAD))
return -EINVAL;
- if (!capable(CAP_SYS_ADMIN))
+ if (!bpf_capable(CAP_SYS_ADMIN))
return -EPERM;
return btf_new_fd(attr);
@@ -2654,7 +2657,7 @@ static int bpf_btf_get_fd_by_id(const union bpf_attr *attr)
if (CHECK_ATTR(BPF_BTF_GET_FD_BY_ID))
return -EINVAL;
- if (!capable(CAP_SYS_ADMIN))
+ if (!bpf_capable(CAP_SYS_ADMIN))
return -EPERM;
return btf_get_fd_by_id(attr->btf_id);
@@ -2723,7 +2726,7 @@ static int bpf_task_fd_query(const union bpf_attr *attr,
if (CHECK_ATTR(BPF_TASK_FD_QUERY))
return -EINVAL;
- if (!capable(CAP_SYS_ADMIN))
+ if (!bpf_capable(CAP_SYS_ADMIN))
return -EPERM;
if (attr->task_fd_query.flags != 0)
@@ -2791,7 +2794,7 @@ SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, siz
union bpf_attr attr = {};
int err;
- if (sysctl_unprivileged_bpf_disabled && !capable(CAP_SYS_ADMIN))
+ if (sysctl_unprivileged_bpf_disabled && !bpf_capable(CAP_SYS_ADMIN))
return -EPERM;
err = bpf_check_uarg_tail_zero(uattr, sizeof(attr), size);
@@ -2886,3 +2889,40 @@ SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, siz
return err;
}
+
+static long bpf_dev_ioctl(struct file *filp,
+ unsigned int ioctl, unsigned long arg)
+{
+ switch (ioctl) {
+ case BPF_DEV_IOCTL_GET_PERM:
+ set_bit(TASK_BPF_FLAG_PERMITTED, ¤t->bpf_flags);
+ break;
+ case BPF_DEV_IOCTL_PUT_PERM:
+ clear_bit(TASK_BPF_FLAG_PERMITTED, ¤t->bpf_flags);
+ break;
+ default:
+ return -EINVAL;
+ }
+ return 0;
+}
+
+static const struct file_operations bpf_chardev_ops = {
+ .unlocked_ioctl = bpf_dev_ioctl,
+};
+
+static struct miscdevice bpf_dev = {
+ .minor = MISC_DYNAMIC_MINOR,
+ .name = "bpf",
+ .fops = &bpf_chardev_ops,
+ .mode = 0440,
+ .nodename = "bpf",
+};
+
+static int __init bpf_dev_init(void)
+{
+ if (misc_register(&bpf_dev))
+ pr_warn("BPF: Failed to create /dev/bpf. Continue without it...\n");
+
+ return 0;
+}
+device_initcall(bpf_dev_init);
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 0e079b2298f8..79dc4d641cf3 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -9134,7 +9134,7 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr,
env->insn_aux_data[i].orig_idx = i;
env->prog = *prog;
env->ops = bpf_verifier_ops[env->prog->type];
- is_priv = capable(CAP_SYS_ADMIN);
+ is_priv = bpf_capable(CAP_SYS_ADMIN);
/* grab the mutex to protect few globals used by verifier */
if (!is_priv)
diff --git a/kernel/bpf/xskmap.c b/kernel/bpf/xskmap.c
index ef7338cebd18..06063679c27a 100644
--- a/kernel/bpf/xskmap.c
+++ b/kernel/bpf/xskmap.c
@@ -21,7 +21,7 @@ static struct bpf_map *xsk_map_alloc(union bpf_attr *attr)
int cpu, err;
u64 cost;
- if (!capable(CAP_NET_ADMIN))
+ if (!bpf_capable(CAP_NET_ADMIN))
return ERR_PTR(-EPERM);
if (attr->max_entries == 0 || attr->key_size != 4 ||
diff --git a/kernel/fork.c b/kernel/fork.c
index 75675b9bf6df..18f914d54d92 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -923,6 +923,10 @@ static struct task_struct *dup_task_struct(struct task_struct *orig, int node)
#ifdef CONFIG_MEMCG
tsk->active_memcg = NULL;
#endif
+
+#ifdef CONFIG_BPF_SYSCALL
+ tsk->bpf_flags = 0;
+#endif
return tsk;
free_stack:
diff --git a/net/core/filter.c b/net/core/filter.c
index 2014d76e0d2a..01ccf031849c 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -5875,7 +5875,7 @@ bpf_base_func_proto(enum bpf_func_id func_id)
break;
}
- if (!capable(CAP_SYS_ADMIN))
+ if (!bpf_capable(CAP_SYS_ADMIN))
return NULL;
switch (func_id) {
@@ -6438,7 +6438,7 @@ static bool cg_skb_is_valid_access(int off, int size,
return false;
case bpf_ctx_range(struct __sk_buff, data):
case bpf_ctx_range(struct __sk_buff, data_end):
- if (!capable(CAP_SYS_ADMIN))
+ if (!bpf_capable(CAP_SYS_ADMIN))
return false;
break;
}
@@ -6450,7 +6450,7 @@ static bool cg_skb_is_valid_access(int off, int size,
case bpf_ctx_range_till(struct __sk_buff, cb[0], cb[4]):
break;
case bpf_ctx_range(struct __sk_buff, tstamp):
- if (!capable(CAP_SYS_ADMIN))
+ if (!bpf_capable(CAP_SYS_ADMIN))
return false;
break;
default:
--
2.17.1
^ permalink raw reply related
* [PATCH bpf-next 4/4] bpftool: use libbpf_[get|put]_bpf_permission()
From: Song Liu @ 2019-06-25 18:23 UTC (permalink / raw)
To: netdev, bpf; +Cc: ast, daniel, kernel-team, Song Liu
In-Reply-To: <20190625182303.874270-1-songliubraving@fb.com>
This patch calls libbpf_[get|put]_bpf_permission() from bpftool. This
allows users with access to /dev/bpf to perform operations like root.
Signed-off-by: Song Liu <songliubraving@fb.com>
---
tools/bpf/bpftool/feature.c | 2 +-
tools/bpf/bpftool/main.c | 5 +++++
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/tools/bpf/bpftool/feature.c b/tools/bpf/bpftool/feature.c
index d672d9086fff..f7f43b91ce96 100644
--- a/tools/bpf/bpftool/feature.c
+++ b/tools/bpf/bpftool/feature.c
@@ -583,7 +583,7 @@ static int do_probe(int argc, char **argv)
/* Detection assumes user has sufficient privileges (CAP_SYS_ADMIN).
* Let's approximate, and restrict usage to root user only.
*/
- if (geteuid()) {
+ if (!libbpf_get_bpf_permission()) {
p_err("please run this command as root user");
return -1;
}
diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
index 4879f6395c7e..f9146d7d8fc5 100644
--- a/tools/bpf/bpftool/main.c
+++ b/tools/bpf/bpftool/main.c
@@ -390,6 +390,10 @@ int main(int argc, char **argv)
if (argc < 0)
usage();
+ if (!libbpf_get_bpf_permission()) {
+ p_err("cannot get permission to access bpf() syscall");
+ usage();
+ }
ret = cmd_select(cmds, argc, argv, do_help);
if (json_output)
@@ -400,5 +404,6 @@ int main(int argc, char **argv)
delete_pinned_obj_table(&map_table);
}
+ libbpf_put_bpf_permission();
return ret;
}
--
2.17.1
^ permalink raw reply related
* [PATCH] xsk: Properly terminate assignment in xskq_produce_flush_desc
From: Nathan Chancellor @ 2019-06-25 18:23 UTC (permalink / raw)
To: Björn Töpel, Magnus Karlsson, David S. Miller,
Alexei Starovoitov, Daniel Borkmann, Jakub Kicinski,
Jesper Dangaard Brouer, John Fastabend
Cc: netdev, bpf, xdp-newbies, linux-kernel, clang-built-linux,
Nick Desaulniers, Nathan Huckleberry, Nathan Chancellor
Clang warns:
In file included from net/xdp/xsk_queue.c:10:
net/xdp/xsk_queue.h:292:2: warning: expression result unused
[-Wunused-value]
WRITE_ONCE(q->ring->producer, q->prod_tail);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/compiler.h:284:6: note: expanded from macro 'WRITE_ONCE'
__u.__val; \
~~~ ^~~~~
1 warning generated.
The q->prod_tail assignment has a comma at the end, not a semi-colon.
Fix that so clang no longer warns and everything works as expected.
Fixes: c497176cb2e4 ("xsk: add Rx receive functions and poll support")
Link: https://github.com/ClangBuiltLinux/linux/issues/544
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---
net/xdp/xsk_queue.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/xdp/xsk_queue.h b/net/xdp/xsk_queue.h
index 88b9ae24658d..cba4a640d5e8 100644
--- a/net/xdp/xsk_queue.h
+++ b/net/xdp/xsk_queue.h
@@ -288,7 +288,7 @@ static inline void xskq_produce_flush_desc(struct xsk_queue *q)
/* Order producer and data */
smp_wmb(); /* B, matches C */
- q->prod_tail = q->prod_head,
+ q->prod_tail = q->prod_head;
WRITE_ONCE(q->ring->producer, q->prod_tail);
}
--
2.22.0
^ permalink raw reply related
* Re: Removing skb_orphan() from ip_rcv_core()
From: Joe Stringer @ 2019-06-25 18:29 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: Joe Stringer, Eric Dumazet, Florian Westphal, netdev,
john fastabend, Daniel Borkmann, Lorenz Bauer, Jakub Sitnicki,
Paolo Abeni
In-Reply-To: <f69a7930-6e8a-d717-0aa4-a63ea6e7b5e0@mojatatu.com>
On Tue, Jun 25, 2019 at 4:07 AM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>
> On 2019-06-24 11:26 p.m., Joe Stringer wrote:
> [..]
> >
> > I haven't got as far as UDP yet, but I didn't see any need for a
> > dependency on netfilter.
>
> I'd be curious to see what you did. My experience, even for TCP is
> the socket(transparent/tproxy) lookup code (to set skb->sk either
> listening or established) is entangled in
> CONFIG_NETFILTER_SOMETHING_OR_OTHER. You have to rip it out of
> there (in the tproxy tc action into that code). Only then can you
> compile out netfilter.
> I didnt bother to rip out code for udp case.
> i.e if you needed udp to work with the tc action,
> youd have to turn on NF. But that was because we had
> no need for udp transparent proxying.
> IOW:
> There is really no reason, afaik, for tproxy code to only be
> accessed if netfilter is compiled in. Not sure i made sense.
Oh, I see. Between the existing bpf_skc_lookup_tcp() and
bpf_sk_lookup_tcp() helpers in BPF, plus a new bpf_sk_assign() helper
and a little bit of lookup code using the appropriate tproxy ports
etc. from the BPF side, I was able to get it working. One could
imagine perhaps wrapping all this logic up in a higher level
"bpf_sk_lookup_tproxy()" helper call or similar, but I didn't go that
direction given that the BPF socket primitives seemed to provide the
necessary functionality in a more generic manner.
^ permalink raw reply
* RE: [PATCH net-next 10/16] qlge: Factor out duplicated expression
From: Manish Chopra @ 2019-06-25 18:32 UTC (permalink / raw)
To: Benjamin Poirier, David Miller; +Cc: GR-Linux-NIC-Dev, netdev@vger.kernel.org
In-Reply-To: <20190624075225.GA27959@f1>
> -----Original Message-----
> From: netdev-owner@vger.kernel.org <netdev-owner@vger.kernel.org> On
> Behalf Of Benjamin Poirier
> Sent: Monday, June 24, 2019 1:22 PM
> To: David Miller <davem@davemloft.net>
> Cc: Manish Chopra <manishc@marvell.com>; GR-Linux-NIC-Dev <GR-Linux-
> NIC-Dev@marvell.com>; netdev@vger.kernel.org
> Subject: Re: [PATCH net-next 10/16] qlge: Factor out duplicated expression
>
> On 2019/06/23 10:59, David Miller wrote:
> > From: Benjamin Poirier <bpoirier@suse.com>
> > Date: Mon, 17 Jun 2019 16:48:52 +0900
> >
> > > Signed-off-by: Benjamin Poirier <bpoirier@suse.com>
> > > ---
> > > drivers/net/ethernet/qlogic/qlge/qlge.h | 6 ++++++
> > > drivers/net/ethernet/qlogic/qlge/qlge_main.c | 18
> > > ++++++------------
> > > 2 files changed, 12 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/drivers/net/ethernet/qlogic/qlge/qlge.h
> > > b/drivers/net/ethernet/qlogic/qlge/qlge.h
> > > index 5a4b2520cd2a..0bb7ccdca6a7 100644
> > > --- a/drivers/net/ethernet/qlogic/qlge/qlge.h
> > > +++ b/drivers/net/ethernet/qlogic/qlge/qlge.h
> > > @@ -77,6 +77,12 @@
> > > #define LSD(x) ((u32)((u64)(x)))
> > > #define MSD(x) ((u32)((((u64)(x)) >> 32)))
> > >
> > > +#define QLGE_FIT16(value) \
> > > +({ \
> > > + typeof(value) _value = value; \
> > > + (_value) == 65536 ? 0 : (u16)(_value); \
> > > +})
> > > +
> >
> > "(u16) 65536" is zero and the range of these values is 0 -- 65536.
> >
> > This whole expression is way overdone.
>
> Indeed, I missed that a simple cast is enough :/
>
> What I inferred from the presence of that expression though is that in the
> places where it is used, the device interprets a value of 0 as 65536. Manish,
> can you confirm that? As David points out, the expression is useless. A
> comment might not be however.
Yes, I think it could be simplified to simple cast just.
^ permalink raw reply
* Re: [PATCH] xsk: Properly terminate assignment in xskq_produce_flush_desc
From: Nick Desaulniers @ 2019-06-25 18:32 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Björn Töpel, Magnus Karlsson, David S. Miller,
Alexei Starovoitov, Daniel Borkmann, Jakub Kicinski,
Jesper Dangaard Brouer, John Fastabend, netdev, bpf, xdp-newbies,
LKML, clang-built-linux, Nathan Huckleberry
In-Reply-To: <20190625182352.13918-1-natechancellor@gmail.com>
On Tue, Jun 25, 2019 at 11:24 AM Nathan Chancellor
<natechancellor@gmail.com> wrote:
>
> Clang warns:
>
> In file included from net/xdp/xsk_queue.c:10:
> net/xdp/xsk_queue.h:292:2: warning: expression result unused
> [-Wunused-value]
> WRITE_ONCE(q->ring->producer, q->prod_tail);
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> include/linux/compiler.h:284:6: note: expanded from macro 'WRITE_ONCE'
> __u.__val; \
> ~~~ ^~~~~
> 1 warning generated.
>
> The q->prod_tail assignment has a comma at the end, not a semi-colon.
> Fix that so clang no longer warns and everything works as expected.
oh no! -Wunderhanded-C-contest-style-use-of-comma-operator strikes again!
Great find and thanks for the fix.
Acked-by: Nick Desaulniers <ndesaulniers@google.com>
Björn and Alexei should carefully review. Because WRITE_ONCE is a
macro that expands to a GNU C statement expression, which returns the
last statement, this code was previously assigning q->prod_tail to
itself, now it's assigning it to q->prod_head. I assume that was not
intentional, but am unfamiliar with the code.
>
> Fixes: c497176cb2e4 ("xsk: add Rx receive functions and poll support")
> Link: https://github.com/ClangBuiltLinux/linux/issues/544
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> ---
> net/xdp/xsk_queue.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/xdp/xsk_queue.h b/net/xdp/xsk_queue.h
> index 88b9ae24658d..cba4a640d5e8 100644
> --- a/net/xdp/xsk_queue.h
> +++ b/net/xdp/xsk_queue.h
> @@ -288,7 +288,7 @@ static inline void xskq_produce_flush_desc(struct xsk_queue *q)
> /* Order producer and data */
> smp_wmb(); /* B, matches C */
>
> - q->prod_tail = q->prod_head,
> + q->prod_tail = q->prod_head;
> WRITE_ONCE(q->ring->producer, q->prod_tail);
> }
>
> --
> 2.22.0
>
--
Thanks,
~Nick Desaulniers
^ permalink raw reply
* Re: [PATCH RFC net-next 1/5] net: dsa: mt7530: Convert to PHYLINK API
From: Daniel Santos @ 2019-06-25 18:37 UTC (permalink / raw)
To: Russell King - ARM Linux admin, René van Dorst
Cc: sean.wang, f.fainelli, davem, matthias.bgg, andrew,
vivien.didelot, frank-w, netdev, linux-mediatek, linux-mips
In-Reply-To: <20190625121030.m5w7wi3rpezhfgyo@shell.armlinux.org.uk>
Hello,
Although I'm new to the entire Ethernet / *MII subsystem and I haven't
touched DSA yet, I've recently had to add some of this functionality to
the older OpenWRT drivers for swconfig control over the ports. René, do
you have an actual datasheet or programming guide for the mt7530? I
only have one for the mt7620.
On 6/25/19 7:10 AM, Russell King - ARM Linux admin wrote:
> mac_link_*().
>
>>>> + if (state->pause || phylink_test(state->advertising, Pause))
>>>> + mcr |= PMCR_TX_FC_EN | PMCR_RX_FC_EN;
>>>> + if (state->pause & MLO_PAUSE_TX)
>>>> + mcr |= PMCR_TX_FC_EN;
>>>> + if (state->pause & MLO_PAUSE_RX)
>>>> + mcr |= PMCR_RX_FC_EN;
>>> This is clearly wrong - if any bit in state->pause is set, then we
>>> end up with both PMCR_TX_FC_EN | PMCR_RX_FC_EN set. If we have Pause
>>> Pause set in the advertising mask, then both are set. This doesn't
>>> seem right - are these bits setting the advertisement, or are they
>>> telling the MAC to use flow control?
>> Last one, tell the MAC to use flow control.
> So the first if() statement is incorrect, and should be removed
> entirely. You only want to enable the MAC to use flow control as a
> result of the negotiation results.
René,
iiuc, this is what's documented in table 28B-3 of the 802.3 spec on page
598. pdf of section 2 here:
http://www.ismlab.usf.edu/dcom/Ch3_802.3-2005_section2.pdf
>> On the current driver both bits are set in a forced-link situation.
>>
>> If we always forces the MAC mode I think I always set these bits and don't
>> anything with the Pause modes? Is that the right way to do it?
> So what happens if your link partner (e.g. switch) does not support
> flow control? What if your link partner floods such frames to all
> ports? You end up transmitting flow control frames, which could be
> sent to all stations on the network... seems not a good idea.
>
> Implementing stuff properly and not taking short-cuts is always a
> good idea for inter-operability.
But will there still be a mechanism to ignore link partner's advertising
and force these parameters? I've run into what appears to be quirks on
two separate NICs or their drivers, a tp-link tg-3468 (r8169) and an
Aquantia AQC107 802.3bz (atlantic) where these link partners aren't
auto-negotiating correctly after I switch the mt7530 out of
auto-negotiation mode. Of course, it could be a mistake I've made (and
should thus be discussed elsewhere), but iirc, I had to force enable
flow control and then also disable auto-negotiation on the link partner
and force the mode I wanted.
Cheers,
Daniel
^ permalink raw reply
* Re: [PATCH 00/11] XDP unaligned chunk placement support
From: Jonathan Lemon @ 2019-06-25 18:44 UTC (permalink / raw)
To: Kevin Laatz
Cc: netdev, ast, daniel, bjorn.topel, magnus.karlsson, bpf,
intel-wired-lan, bruce.richardson, ciara.loftus
In-Reply-To: <20190620083924.1996-1-kevin.laatz@intel.com>
On 20 Jun 2019, at 1:39, Kevin Laatz wrote:
> This patchset adds the ability to use unaligned chunks in the XDP
> umem.
>
> Currently, all chunk addresses passed to the umem are masked to be
> chunk
> size aligned (default is 2k, max is PAGE_SIZE). This limits where we
> can
> place chunks within the umem as well as limiting the packet sizes that
> are
> supported.
>
> The changes in this patchset removes these restrictions, allowing XDP
> to be
> more flexible in where it can place a chunk within a umem. By relaxing
> where
> the chunks can be placed, it allows us to use an arbitrary buffer size
> and
> place that wherever we have a free address in the umem. These changes
> add the
> ability to support jumboframes and make it easy to integrate with
> other
> existing frameworks that have their own memory management systems,
> such as
> DPDK.
I'm a little unclear on how this should work, and have a few issues
here:
1) There isn't any support for the user defined umem->headroom
2) When queuing RX buffers, the handle (aka umem offset) is used,
which
points to the start of the buffer area. When the buffer appears in
the completion queue, handle points to the start of the received
data,
which might be different from the buffer start address.
Normally, this RX address is just put back in the fill queue, and
the
mask is used to find the buffer start address again. This no
longer
works, so my question is, how is the buffer start address
recomputed
from the actual data payload address?
Same with TX - if the TX payload isn't aligned in with the start of
the buffer, what happens?
3) This appears limited to crossing a single page boundary, but there
is no constraint check on chunk_size.
--
Jonathan
>
> Structure of the patchset:
> Patch 1:
> - Remove unnecessary masking and headroom addition during zero-copy
> Rx
> buffer recycling in i40e. This change is required in order for the
> buffer recycling to work in the unaligned chunk mode.
>
> Patch 2:
> - Remove unnecessary masking and headroom addition during
> zero-copy Rx buffer recycling in ixgbe. This change is required in
> order for the buffer recycling to work in the unaligned chunk
> mode.
>
> Patch 3:
> - Adds an offset parameter to zero_copy_allocator. This change will
> enable us to calculate the original handle in zca_free. This will
> be
> required for unaligned chunk mode since we can't easily mask back
> to
> the original handle.
>
> Patch 4:
> - Adds the offset parameter to i40e_zca_free. This change is needed
> for
> calculating the handle since we can't easily mask back to the
> original
> handle like we can in the aligned case.
>
> Patch 5:
> - Adds the offset parameter to ixgbe_zca_free. This change is needed
> for
> calculating the handle since we can't easily mask back to the
> original
> handle like we can in the aligned case.
>
>
> Patch 6:
> - Add infrastructure for unaligned chunks. Since we are dealing
> with unaligned chunks that could potentially cross a physical page
> boundary, we add checks to keep track of that information. We can
> later use this information to correctly handle buffers that are
> placed at an address where they cross a page boundary.
>
> Patch 7:
> - Add flags for umem configuration to libbpf
>
> Patch 8:
> - Modify xdpsock application to add a command line option for
> unaligned chunks
>
> Patch 9:
> - Addition of command line argument to pass in a desired buffer size
> and buffer recycling for unaligned mode. Passing in a buffer size
> will
> allow the application to use unaligned chunks with the unaligned
> chunk
> mode. Since we are now using unaligned chunks, we need to recycle
> our
> buffers in a slightly different way.
>
> Patch 10:
> - Adds hugepage support to the xdpsock application
>
> Patch 11:
> - Documentation update to include the unaligned chunk scenario. We
> need
> to explicitly state that the incoming addresses are only masked in
> the
> aligned chunk mode and not the unaligned chunk mode.
>
> Kevin Laatz (11):
> i40e: simplify Rx buffer recycle
> ixgbe: simplify Rx buffer recycle
> xdp: add offset param to zero_copy_allocator
> i40e: add offset to zca_free
> ixgbe: add offset to zca_free
> xsk: add support to allow unaligned chunk placement
> libbpf: add flags to umem config
> samples/bpf: add unaligned chunks mode support to xdpsock
> samples/bpf: add buffer recycling for unaligned chunks to xdpsock
> samples/bpf: use hugepages in xdpsock app
> doc/af_xdp: include unaligned chunk case
>
> Documentation/networking/af_xdp.rst | 10 +-
> drivers/net/ethernet/intel/i40e/i40e_xsk.c | 21 ++--
> drivers/net/ethernet/intel/i40e/i40e_xsk.h | 3 +-
> .../ethernet/intel/ixgbe/ixgbe_txrx_common.h | 3 +-
> drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c | 21 ++--
> include/net/xdp.h | 3 +-
> include/net/xdp_sock.h | 2 +
> include/uapi/linux/if_xdp.h | 4 +
> net/core/xdp.c | 11 ++-
> net/xdp/xdp_umem.c | 17 ++--
> net/xdp/xsk.c | 60 +++++++++--
> net/xdp/xsk_queue.h | 60 +++++++++--
> samples/bpf/xdpsock_user.c | 99
> ++++++++++++++-----
> tools/include/uapi/linux/if_xdp.h | 4 +
> tools/lib/bpf/xsk.c | 7 ++
> tools/lib/bpf/xsk.h | 2 +
> 16 files changed, 241 insertions(+), 86 deletions(-)
>
> --
> 2.17.1
^ permalink raw reply
* Re: [PATCH V3 04/15] ARM: exynos: cleanup cppcheck shifting error
From: Krzysztof Kozlowski @ 2019-06-25 18:49 UTC (permalink / raw)
To: Phong Tran
Cc: acme, alexander.shishkin, alexander.sverdlin, allison, andrew,
ast, bgolaszewski, bpf, daniel, daniel, dmg, festevam, gerg,
gregkh, gregory.clement, haojian.zhuang, hsweeten,
illusionist.neo, info, jason, jolsa, kafai, kernel, kgene,
kstewart, linux-arm-kernel, linux-imx, linux-kernel, linux-omap,
linux-samsung-soc, linux, liviu.dudau, lkundrak,
lorenzo.pieralisi, mark.rutland, mingo, namhyung, netdev, nsekhar,
peterz, robert.jarzmik, s.hauer, sebastian.hesselbarth, shawnguo,
songliubraving, sudeep.holla, swinslow, tglx, tony, will, yhs
In-Reply-To: <20190625040356.27473-5-tranmanphong@gmail.com>
On Tue, Jun 25, 2019 at 11:03:45AM +0700, Phong Tran wrote:
> There is error from cppcheck tool
> "Shifting signed 32-bit value by 31 bits is undefined behaviour errors"
> change to use BIT() marco for improvement.
>
> Signed-off-by: Phong Tran <tranmanphong@gmail.com>
> ---
> arch/arm/mach-exynos/suspend.c | 2 +-
Thanks, applied with slightly different commit message. As Peter
pointed, there is no error because of GCC. Usually we expect a reply to
comments on LKML... and also you could take his hints and use them to
improve the commit msg to properly describe what is the problem.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH iproute2 1/2] devlink: fix format string warning for 32bit targets
From: Stephen Hemminger @ 2019-06-25 18:58 UTC (permalink / raw)
To: Baruch Siach; +Cc: Jiri Pirko, netdev, Aya Levin, Moshe Shemesh
In-Reply-To: <016aabe2639668b4710b73157ea39e8f97f7d726.1561463345.git.baruch@tkos.co.il>
On Tue, 25 Jun 2019 14:49:04 +0300
Baruch Siach <baruch@tkos.co.il> wrote:
> diff --git a/devlink/devlink.c b/devlink/devlink.c
> index 436935f88bda..b400fab17578 100644
> --- a/devlink/devlink.c
> +++ b/devlink/devlink.c
> @@ -1726,9 +1726,9 @@ static void pr_out_u64(struct dl *dl, const char *name, uint64_t val)
> jsonw_u64_field(dl->jw, name, val);
> } else {
> if (g_indent_newline)
> - pr_out("%s %lu", name, val);
> + pr_out("%s %llu", name, val);
> else
> - pr_out(" %s %lu", name, val);
> + pr_out(" %s %llu", name, val);
But on 64 bit target %llu expects unsigned long long which is 128bit.
The better way to fix this is to use:
#include <inttypes.h>
And the use the macro PRIu64
pr_out(" %s %"PRIu64, name, val);
^ permalink raw reply
* Re: [PATCH iproute2 2/2] devlink: fix libc and kernel headers collision
From: Stephen Hemminger @ 2019-06-25 18:59 UTC (permalink / raw)
To: Baruch Siach; +Cc: Jiri Pirko, netdev, Aya Levin, Moshe Shemesh
In-Reply-To: <7f81026b803edcaeaca05994298118271a2f287d.1561463345.git.baruch@tkos.co.il>
On Tue, 25 Jun 2019 14:49:05 +0300
Baruch Siach <baruch@tkos.co.il> wrote:
> +/* Suppress linux/sysinfo.h to avoid
> + * collision of struct sysinfo definition
> + * with musl libc headers
> + */
You only need a small comment for this.
#define _LINUX_SYSINFO_H /* avoid collision with musl header */
^ permalink raw reply
* Re: [PATCH RFC net-next 1/5] net: dsa: mt7530: Convert to PHYLINK API
From: Andrew Lunn @ 2019-06-25 19:02 UTC (permalink / raw)
To: Daniel Santos
Cc: Russell King - ARM Linux admin, René van Dorst, sean.wang,
f.fainelli, davem, matthias.bgg, vivien.didelot, frank-w, netdev,
linux-mediatek, linux-mips
In-Reply-To: <1ad9f9a5-8f39-40bd-94bb-6b700f30c4ba@pobox.com>
> But will there still be a mechanism to ignore link partner's advertising
> and force these parameters?
From man 1 ethtool:
-a --show-pause
Queries the specified Ethernet device for pause parameter information.
-A --pause
Changes the pause parameters of the specified Ethernet device.
autoneg on|off
Specifies whether pause autonegotiation should be enabled.
rx on|off
Specifies whether RX pause should be enabled.
tx on|off
Specifies whether TX pause should be enabled.
You need to check the driver to see if it actually implements this
ethtool call, but that is how it should be configured.
Andrew
^ permalink raw reply
* Re: [PATCH v4 3/7] lib/hexdump.c: Optionally suppress lines of repeated bytes
From: kbuild test robot @ 2019-06-25 19:13 UTC (permalink / raw)
To: Alastair D'Silva
Cc: kbuild-all, alastair, linux-fbdev, Stanislaw Gruszka, Petr Mladek,
David Airlie, Joonas Lahtinen, dri-devel, devel, linux-scsi,
Jassi Brar, ath10k, intel-gfx, Dan Carpenter, Jose Abreu,
Tom Lendacky, James E.J. Bottomley, Jani Nikula, linux-fsdevel,
Steven Rostedt, Rodrigo Vivi, Benson Leung, Kalle Valo,
Karsten Keil, Martin K. Petersen, Greg Kroah-Hartman,
linux-wireless, linux-kernel, Sergey Senozhatsky, David Laight,
Daniel Vetter, netdev, Enric Balletbo i Serra, Andrew Morton,
David S. Miller, Alexander Viro
In-Reply-To: <20190625031726.12173-4-alastair@au1.ibm.com>
[-- Attachment #1: Type: text/plain, Size: 1676 bytes --]
Hi Alastair,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[also build test WARNING on v5.2-rc6 next-20190625]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Alastair-D-Silva/Hexdump-Enhancements/20190625-224046
config: xtensa-allyesconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 7.4.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.4.0 make.cross ARCH=xtensa
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
lib/hexdump.c: In function 'announce_skipped':
>> lib/hexdump.c:243:28: warning: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type 'size_t {aka unsigned int}' [-Wformat=]
printk("%s%s ** Skipped %lu bytes of value 0x%x **\n",
~~^
%u
vim +243 lib/hexdump.c
236
237 static void announce_skipped(const char *level, const char *prefix_str,
238 u8 val, size_t count)
239 {
240 if (count == 0)
241 return;
242
> 243 printk("%s%s ** Skipped %lu bytes of value 0x%x **\n",
244 level, prefix_str, count, val);
245 }
246
---
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: 58637 bytes --]
^ 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