Netdev List
 help / color / mirror / Atom feed
* Re: pull-request: ieee802154 2018-05-08
From: David Miller @ 2018-05-09  0:06 UTC (permalink / raw)
  To: stefan; +Cc: s.schmidt, linux-wpan, alex.aring, netdev
In-Reply-To: <a61fa7c3-fa57-535c-7f00-b495d0a46bb0@osg.samsung.com>

From: Stefan Schmidt <stefan@osg.samsung.com>
Date: Tue, 8 May 2018 21:55:37 +0200

> On 05/08/2018 04:18 PM, David Miller wrote:
>> Please submit the -stable fix directly, you can feel free to CC: me.
> 
> Will do when the patch hits Linus git tree.
> 
> I have a quick question on the process here. From the netdev-faq document
> I was under the impression all stable patches under net/ and drivers/net
> should be brought up to you and would be handled by you.
> 
> Does this apply to the core part of net (I fully understand that ieee802154
> is rather a niche) or is there some other reason for this exception?
> 
> Both processes (the normal stable one as well as the slightly different one
> for net/) would be fine to go with for me. Just need to know which one I
> should use for future stable patches. :-)
> 
> regards
> Stefan Schmidt

Generally wireless and ipsec have been submitting them directly,
sometimes the Intel ethernet guys do too.

Sometimes this makes things easier for me, and I'll ask you to submit
things directly when that is the case like right now.

Thank you.

^ permalink raw reply

* Re: Failed to clone net-next.git
From: David Miller @ 2018-05-09  0:04 UTC (permalink / raw)
  To: songliubraving; +Cc: netdev, ast, konstantin
In-Reply-To: <FB252D9C-2FB2-437C-8646-4F61406AC96D@fb.com>

From: Song Liu <songliubraving@fb.com>
Date: Tue, 8 May 2018 17:46:23 +0000

> We are seeing the following error on multiple different systems while 
> cloning net-next tree. 
> 
>   $ git clone https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git
>   Cloning into 'net-next'...

Regardless of the failure, it is so _insanely_ wasteful to clone my
trees like this.

Just simply always have Linus's tree always checked out somewhere:

	git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git

Let's say you have it under src/GIT/linux as I do.

Then go to src/GIT and say:

	git clone --reference linux/.git https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git

This way it only downloads the objects that are unique to the net-next
tree.  Similarly for 'net':

	git clone --reference linux/.git https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git

Or any other subsystem tree.

Periodically "git pull --ff-only" your Linus's tree, and you'll be
much happier in GIT land :-)

As subsystem changes make their way into Linus's GIT tree, git will
notice over time and garbage collect the dups that are in your
subsystem GIT trees.

^ permalink raw reply

* Re: [bpf-next v2 0/9] bpf: Add helper to do FIB lookups
From: Daniel Borkmann @ 2018-05-08 23:53 UTC (permalink / raw)
  To: David Ahern, netdev, borkmann, ast
  Cc: davem, shm, roopa, brouer, toke, john.fastabend
In-Reply-To: <20180504025432.23451-1-dsahern@gmail.com>

On 05/04/2018 04:54 AM, David Ahern wrote:
> Provide a helper for doing a FIB and neighbor lookup in the kernel
> tables from an XDP program. The helper provides a fastpath for forwarding
> packets. If the packet is a local delivery or for any reason is not a
> simple lookup and forward, the packet is expected to continue up the stack
> for full processing.
> 
> The response from a FIB and neighbor lookup is either the egress index
> with the bpf_fib_lookup struct filled in with dmac and gateway or
> 0 meaning the packet should continue up the stack. In time we can
> revisit this to return the FIB lookup result errno if it is one of the
> special RTN_'s such as RTN_BLACKHOLE (-EINVAL) so that the XDP
> programs can do an early drop if desired.
> 
> Patches 1-6 do some more refactoring to IPv6 with the end goal of
> extracting a FIB lookup function that aligns with fib_lookup for IPv4,
> basically returning a fib6_info without creating a dst based entry.
> 
> Patch 7 adds lookup functions to the ipv6 stub. These are needed since
> bpf is built into the kernel and ipv6 may not be built or loaded.
> 
> Patch 8 adds the bpf helper and 9 adds a sample program.
> 
> v2
> - removed pkt_access from bpf_func_proto as noticed by Daniel
> - added check in that IPv6 forwarding is enabled 
> - added DaveM's ack on patches 1-7 and 9 based on v1 response and
>   fact that no changes were made to them in v2
> 
> v1
> - updated commit messages and cover letter
> - added comment to sample program noting lack of verification on
>   egress device supporting XDP
> 
> RFC v2
> - fixed use of foward helper from cls_act as noted by Daniel
> - in patch 1 rename fib6_lookup_1 as well for consistency

Applied to bpf-next, thanks David.

^ permalink raw reply

* Re: [PATCH] hv_netvsc: Fix net device attach on older Windows hosts
From: Stephen Hemminger @ 2018-05-08 22:49 UTC (permalink / raw)
  To: Mohammed Gamal; +Cc: sthemmin, netdev, haiyangz, linux-kernel, devel, vkuznets
In-Reply-To: <1525803471.4366.15.camel@redhat.com>

On Tue, 08 May 2018 20:17:51 +0200
Mohammed Gamal <mgamal@redhat.com> wrote:

> On Tue, 2018-05-08 at 11:13 -0700, Stephen Hemminger wrote:
> > On Tue,  8 May 2018 19:40:47 +0200
> > Mohammed Gamal <mgamal@redhat.com> wrote:
> >   
> > > On older windows hosts the net_device instance is returned to
> > > the caller of rndis_filter_device_add() without having the presence
> > > bit set first. This would cause any subsequent calls to network
> > > device
> > > operations (e.g. MTU change, channel change) to fail after the
> > > device
> > > is detached once, returning -ENODEV.
> > > 
> > > Make sure we explicitly call netif_device_attach() before returning
> > > the net_device instance to make sure the presence bit is set
> > > 
> > > Fixes: 7b2ee50c0cd5 ("hv_netvsc: common detach logic")
> > > 
> > > Signed-off-by: Mohammed Gamal <mgamal@redhat.com>
> > > ---
> > >  drivers/net/hyperv/rndis_filter.c | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/net/hyperv/rndis_filter.c
> > > b/drivers/net/hyperv/rndis_filter.c
> > > index 6b127be..09a3c1d 100644
> > > --- a/drivers/net/hyperv/rndis_filter.c
> > > +++ b/drivers/net/hyperv/rndis_filter.c
> > > @@ -1287,8 +1287,10 @@ struct netvsc_device
> > > *rndis_filter_device_add(struct hv_device *dev,
> > >  		   rndis_device->hw_mac_adr,
> > >  		   rndis_device->link_state ? "down" : "up");
> > >  
> > > -	if (net_device->nvsp_version < NVSP_PROTOCOL_VERSION_5)
> > > +	if (net_device->nvsp_version < NVSP_PROTOCOL_VERSION_5) {
> > > +		netif_device_attach(net);
> > >  		return net_device;
> > > +	}  
> > 
> > Yes, this looks right, but it might be easier to use goto existing
> > exit
> > path.
> >   
> 
> I was just not sure if we should set max_chn and num_chn here. I will
> modify the patch and resend.


On older code it was just a goto. At that point: num_chn = max_chn = 1
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply

* Re: The SO_BINDTODEVICE was set to the desired interface, but packets are received from all interfaces.
From: David Ahern @ 2018-05-08 22:48 UTC (permalink / raw)
  To: Paolo Abeni, Damir Mansurov, netdev
  Cc: Konstantin Ushakov, Alexandra N. Kossovsky, Andrey Dmitrov
In-Reply-To: <1525696890.2587.24.camel@redhat.com>

On 5/7/18 6:41 AM, Paolo Abeni wrote:
> Hi,
> On Mon, 2018-05-07 at 13:19 +0300, Damir Mansurov wrote:
>> After successful call of the setsockopt(SO_BINDTODEVICE) function to set 
>> data reception from only one interface, the data is still received from 
>> all interfaces. Function setsockopt() returns 0 but then recv() receives 
>> data from all available network interfaces.
>>
>> The problem is reproducible on linux kernels 4.14 - 4.16, but it does 
>> not on linux kernels 4.4, 4.13.
> 
> I think that the cause is commit:
> 
> commit fb74c27735f0a34e76dbf1972084e984ad2ea145
> Author: David Ahern <dsahern@gmail.com>
> Date:   Mon Aug 7 08:44:16 2017 -0700
> 
>     net: ipv4: add second dif to udp socket lookups
> 
> Something like the following should fix, but I'm unsure it preserves
> the intended semathics for 'sdif'. David, can you please have a look?
> Thanks!
> 
> Paolo
> ---
> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> index dd3102a37ef9..0d593d5c33cf 100644
> --- a/net/ipv4/udp.c
> +++ b/net/ipv4/udp.c
> @@ -401,9 +401,9 @@ static int compute_score(struct sock *sk, struct net *net,
>  		bool dev_match = (sk->sk_bound_dev_if == dif ||
>  				  sk->sk_bound_dev_if == sdif);
>  
> -		if (exact_dif && !dev_match)
> +		if (!dev_match)
>  			return -1;
> -		if (sk->sk_bound_dev_if && dev_match)
> +		if (sk->sk_bound_dev_if)
>  			score += 4;
>  	}
>  
> 

The above fixes the reported problem. You should make the same change to
ipv6 as well. Fixes tags:

Fixes: fb74c27735f0a ("net: ipv4: add second dif to udp socket lookups")
Fixes: 1801b570dd2ae ("net: ipv6: add second dif to udp socket lookups")


The change does break a VRF use case, but that case works by accident
given this bug. The use case is a client or server bound to an enslaved
device and trying to communicate locally. In some cases the error is the
ICMP 'Connection refused' getting lost; in other cases the packets don't
make it from one scope to another (eg., VRF based server talking to
device based client). After poking around for a couple of days, I
believe the proper fix for this uses case is beyond the scope of
anything that should be backported to 4.14. So I am fine with the
breakage to what is IMHO a corner case - and there is a reasonable
workaround until I find a proper solution.

^ permalink raw reply

* Re: [PATCH v6 00/13] firmware_loader changes for v4.18
From: Kees Cook @ 2018-05-08 22:45 UTC (permalink / raw)
  To: Luis R. Rodriguez
  Cc: Greg KH, Andrew Morton, Josh Triplett, maco, Andy Gross,
	David Brown, bjorn.andersson, teg, wagi, Hans de Goede,
	Andres Rodriguez, Mimi Zohar, Jakub Kicinski, Shuah Khan,
	Martin Fuzzey, David Howells, pali.rohar, Takashi Iwai,
	Kalle Valo, Arend van Spriel, Rafał Miłecki,
	Nicolas Broeking
In-Reply-To: <20180508181247.19431-1-mcgrof@kernel.org>

On Tue, May 8, 2018 at 11:12 AM, Luis R. Rodriguez <mcgrof@kernel.org> wrote:
> Greg,
>
> Here is what I have queued up for the firmware_loader for v4.18. It
> includes a slew of cleanup work, and the new firmware_request_nowarn()
> which is quiet but enables the sysfs fallback mechanism. I've gone ahead
> and also queued up a few minor fixes for the firmware loader documentation
> which have come up recently. These changes are available on my git tree
> both based on linux-next [0] and Linus' latest tree [1]. Folks working
> on new developments for the firmware loader can use my linux-next
> branch 20180508-firmware_loader_for-v4.18-try2 for now.
>
> 0-day sends its blessings.
>
> The patches from Mimi's series still require a bit more discussion and
> review. The discussion over the EFI firmware fallback mechanism is still
> ongoing.
>
> As for the rename that you wanted, perhaps we can do this late in the
> merge window considering we're at rc4 now. I can prep something up for
> that later.
>
> Question, and specially rants are warmly welcomed.

I sent some typo catches, but with those fixed, please consider the
whole series:

Reviewed-by: Kees Cook <keescook@chromium.org>

Thanks!

-Kees

-- 
Kees Cook
Pixel Security

^ permalink raw reply

* Re: [PATCH v6 05/13] firmware_loader: enhance Kconfig documentation over FW_LOADER
From: Kees Cook @ 2018-05-08 22:42 UTC (permalink / raw)
  To: Luis R. Rodriguez
  Cc: Greg KH, Andrew Morton, Josh Triplett, maco, Andy Gross,
	David Brown, bjorn.andersson, teg, wagi, Hans de Goede,
	Andres Rodriguez, Mimi Zohar, Jakub Kicinski, Shuah Khan,
	Martin Fuzzey, David Howells, pali.rohar, Takashi Iwai,
	Kalle Valo, Arend van Spriel, Rafał Miłecki,
	Nicolas Broeking
In-Reply-To: <20180508181247.19431-6-mcgrof@kernel.org>

On Tue, May 8, 2018 at 11:12 AM, Luis R. Rodriguez <mcgrof@kernel.org> wrote:
> If you try to read FW_LOADER today it speaks of old riddles and
> unless you have been following development closely you will loose

Typo: lose

> track of what is what. Even the documentation for PREVENT_FIRMWARE_BUILD
> is a bit fuzzy and how it fits into this big picture.
>
> Give the FW_LOADER kconfig documentation some love with more up to
> date developments and recommendations. While at it, wrap the FW_LOADER
> code into its own menu to compartamentalize and make it clearer which

Typo: compartmentalize

> components really are part of the FW_LOADER. This should also make
> it easier to later move these kconfig entries into the firmware_loader/
> directory later.
>
> This also now recommends using firmwared [0] for folks left needing a uevent
> handler in userspace for the sysfs firmware fallback mechanis given udev's
> uevent firmware mechanism was ripped out a while ago.
>
> [0] https://github.com/teg/firmwared
>
> Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
> ---
>  drivers/base/Kconfig | 165 ++++++++++++++++++++++++++++++++++---------
>  1 file changed, 131 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
> index 29b0eb452b3a..a4fe86caecca 100644
> --- a/drivers/base/Kconfig
> +++ b/drivers/base/Kconfig
> @@ -70,39 +70,64 @@ config STANDALONE
>           If unsure, say Y.
>
>  config PREVENT_FIRMWARE_BUILD
> -       bool "Prevent firmware from being built"
> +       bool "Disable drivers features which enable custom firmware building"
>         default y
>         help
> -         Say yes to avoid building firmware. Firmware is usually shipped
> -         with the driver and only when updating the firmware should a
> -         rebuild be made.
> -         If unsure, say Y here.
> +         Say yes to disable driver features which enable building a custom
> +         driver firmwar at kernel build time. These drivers do not use the

Typo: firmware

> +         kernel firmware API to load firmware (CONFIG_FW_LOADER), instead they
> +         use their own custom loading mechanism. The required firmware is
> +         usually shipped with the driver, building the driver firmware
> +         should only be needed if you have an updated firmware source.
> +
> +         Firmware should not be being built as part of kernel, these days
> +         you should always prevent this and say Y here. There are only two
> +         old drivers which enable building of its firmware at kernel build
> +         time:
> +
> +           o CONFIG_WANXL through CONFIG_WANXL_BUILD_FIRMWARE
> +           o CONFIG_SCSI_AIC79XX through CONFIG_AIC79XX_BUILD_FIRMWARE
> +
> +menu "Firmware loader"
>
>  config FW_LOADER
> -       tristate "Userspace firmware loading support" if EXPERT
> +       tristate "Firmware loading facility" if EXPERT
>         default y
>         ---help---
> -         This option is provided for the case where none of the in-tree modules
> -         require userspace firmware loading support, but a module built
> -         out-of-tree does.
> +         This enables the firmware loading facility in the kernel. The kernel
> +         will first look for built-in firmware, if it has any. Next, it will
> +         look for the requested firmware in a series of filesystem paths:
> +
> +               o firmware_class path module parameter or kernel boot param
> +               o /lib/firmware/updates/UTS_RELEASE
> +               o /lib/firmware/updates
> +               o /lib/firmware/UTS_RELEASE
> +               o /lib/firmware
> +
> +         Enabling this feature only increases your kernel image by about
> +         828 bytes, enable this option unless you are certain you don't
> +         need firmware.
> +
> +         You typically want this built-in (=y) but you can also enable this
> +         as a module, in which case the firmware_class module will be built.
> +         You also want to be sure to enable this built-in if you are going to
> +         enable built-in firmware (CONFIG_EXTRA_FIRMWARE).
> +
> +if FW_LOADER
>
>  config EXTRA_FIRMWARE
> -       string "External firmware blobs to build into the kernel binary"
> -       depends on FW_LOADER
> +       string "Build these firmware blobs into the kernel binary"

Maybe "Build named firmware blobs ..." "these" took me a while to figure out.

>         help
> -         Various drivers in the kernel source tree may require firmware,
> -         which is generally available in your distribution's linux-firmware
> -         package.
> +         Device drivers which require firmware can typically deal with
> +         having the kernel load firmware from the various supported
> +         /lib/firmware/ paths. This option enables you to build into the
> +         kernel firmware files. Built-in firmware searches are preceeded

Typo: preceded

> +         over firmware lookups using your filesystem over the supported
> +         /lib/firmware paths documented on CONFIG_FW_LOADER.
>
> -         The linux-firmware package should install firmware into
> -         /lib/firmware/ on your system, so they can be loaded by userspace
> -         helpers on request.
> -
> -         This option allows firmware to be built into the kernel for the case
> -         where the user either cannot or doesn't want to provide it from
> -         userspace at runtime (for example, when the firmware in question is
> -         required for accessing the boot device, and the user doesn't want to
> -         use an initrd).
> +         This may be useful for testing or if the firmware is required early on
> +         in boot and cannot rely on the firmware being placed in an initrd or
> +         initramfs.
>
>           This option is a string and takes the (space-separated) names of the
>           firmware files -- the same names that appear in MODULE_FIRMWARE()
> @@ -113,7 +138,7 @@ config EXTRA_FIRMWARE
>           For example, you might set CONFIG_EXTRA_FIRMWARE="usb8388.bin", copy
>           the usb8388.bin file into /lib/firmware, and build the kernel. Then
>           any request_firmware("usb8388.bin") will be satisfied internally
> -         without needing to call out to userspace.
> +         inside the kernel without ever looking at your filesystem at runtime.
>
>           WARNING: If you include additional firmware files into your binary
>           kernel image that are not available under the terms of the GPL,
> @@ -130,22 +155,94 @@ config EXTRA_FIRMWARE_DIR
>           looks for the firmware files listed in the EXTRA_FIRMWARE option.
>
>  config FW_LOADER_USER_HELPER
> -       bool
> +       bool "Enable the firmware sysfs fallback mechanism"
> +       help
> +         This option enables a sysfs loading facility to enable firmware
> +         loading to the kernel through userspace as a fallback mechanism
> +         if and only if the kernel's direct filesystem lookup for the
> +         firmware failed using the different /lib/firmware/ paths, or the
> +         path specified in the firmware_class path module parameter, or the
> +         firmware_class path kernel boot parameter if the firmware_class is
> +         built-in. For details on how to work with the sysfs fallback mechanism
> +         refer to Documentation/driver-api/firmware/fallback-mechanisms.rst.
> +
> +         The direct filesystem lookup for firwmare is always used first now.

Typo: firmware

> +
> +         If the kernel's direct filesystem lookup for firware fails to find
> +         the requested firmware a sysfs fallback loading facility is made
> +         available and userspace is informed about this through uevents.
> +         The uevent can be supressed if the driver explicitly requested it,
> +         this is known as the driver using the custom fallback mechanism.
> +         If the custom fallback mechanism is used userspace must always
> +         acknowledge failure to find firmware as the timeout for the fallback
> +         mechanism is disabled, and failed requests will linger forever.
> +
> +         This used to be the default firmware loading facility, and udev used
> +         to listen for uvents to load firmware for the kernel. The firmware
> +         loading facility functionality in udev has been removed, as such it
> +         can no longer be relied upon as a fallback mechanism. Linux no longer
> +         relies on or uses a fallback mechanism in userspace. If you need to
> +         rely on one refer to the permissively licensed firmwared:

Typo: firmware

> +
> +         https://github.com/teg/firmwared
> +
> +         Since this was the default firmware loading facility at one point,
> +         old userspace may exist which relies upon it, and as such this
> +         mechanism can never be removed from the kernel.
> +
> +         You should only enable this functionality if you are certain you
> +         require a fallback mechanism and have a userspace mechanism ready to
> +         load firmware in case it is not found. One main reason for this may
> +         be if you have drivers which require firmware built-in and for
> +         whatever reason cannot place the required firmware in initramfs.
> +         Another reason kernels may have this feature enabled is to support a
> +         driver which explicitly relies on this fallback mechanism. Only two
> +         drivers need this today:
> +
> +           o CONFIG_LEDS_LP55XX_COMMON
> +           o CONFIG_DELL_RBU
> +
> +         Outside of supporting the above drivers, another reason for needing
> +         this may be that your firmware resides outside of the paths the kernel
> +         looks for and cannot possibily be specified using the firmware_class
> +         path module parameter or kernel firmware_class path boot parameter
> +         if firmware_class is built-in.
> +
> +         A modern use case may be to temporarily mount a custom partition
> +         during provisioning which is only accessible to userspace, and then
> +         to use it to look for and fetch the required firmware. Such type of
> +         driver functionality may not even ever be desirable upstream by
> +         vendors, and as such is only required to be supported as an interface
> +         for provisioning. Since udev's firmware loading facility has been
> +         removed you can use firmwared or a fork of it to customize how you
> +         want to load firmware based on uevents issued.
> +
> +         Enabling this option will increase your kernel image size by about
> +         13436 bytes.
> +
> +         If you are unsure about this, say N here, unless you are Linux
> +         distribution and need to support the above two drivers, or you are
> +         certain you need to support some really custom firmware loading
> +         facility in userspace.
>
>  config FW_LOADER_USER_HELPER_FALLBACK
> -       bool "Fallback user-helper invocation for firmware loading"
> -       depends on FW_LOADER
> -       select FW_LOADER_USER_HELPER
> +       bool "Force the firmware sysfs fallback mechanism when possible"
> +       depends on FW_LOADER_USER_HELPER
>         help
> -         This option enables / disables the invocation of user-helper
> -         (e.g. udev) for loading firmware files as a fallback after the
> -         direct file loading in kernel fails.  The user-mode helper is
> -         no longer required unless you have a special firmware file that
> -         resides in a non-standard path. Moreover, the udev support has
> -         been deprecated upstream.
> +         Enabling this option forces a sysfs userspace fallback mechanism
> +         to be used for all firmware requests which explicitly do not disable a
> +         a fallback mechanism. Firmware calls which do prohibit a fallback
> +         mechanism is request_firmware_direct(). This option is kept for
> +          backward compatibility purposes given this precise mechanism can also
> +         be enabled by setting the proc sysctl value to true:
> +
> +              /proc/sys/kernel/firmware_config/force_sysfs_fallback
>
>           If you are unsure about this, say N here.
>
> +endif # FW_LOADER
> +endmenu
> +
>  config WANT_DEV_COREDUMP
>         bool
>         help
> --
> 2.17.0
>


-Kees

-- 
Kees Cook
Pixel Security

^ permalink raw reply

* [PATCH] sctp: fix spelling mistake: "max_retans" -> "max_retrans"
From: Colin King @ 2018-05-08 22:24 UTC (permalink / raw)
  To: Vlad Yasevich, Neil Horman, Marcelo Ricardo Leitner,
	David S . Miller, linux-sctp, netdev
  Cc: kernel-janitors, linux-kernel

From: Colin Ian King <colin.king@canonical.com>

Trivial fix to spelling mistake in error string

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 net/sctp/sm_make_chunk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index 4d7b3ccea078..4a4fd1971255 100644
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -1131,7 +1131,7 @@ struct sctp_chunk *sctp_make_violation_max_retrans(
 					const struct sctp_association *asoc,
 					const struct sctp_chunk *chunk)
 {
-	static const char error[] = "Association exceeded its max_retans count";
+	static const char error[] = "Association exceeded its max_retrans count";
 	size_t payload_len = sizeof(error) + sizeof(struct sctp_errhdr);
 	struct sctp_chunk *retval;
 
-- 
2.17.0

^ permalink raw reply related

* [PATCH] [ATM] firestream: fix spelling mistake: "reseverd" -> "reserved"
From: Colin King @ 2018-05-08 22:01 UTC (permalink / raw)
  To: Chas Williams, linux-atm-general, netdev; +Cc: kernel-janitors, linux-kernel

From: Colin Ian King <colin.king@canonical.com>

Trivial fix to spelling mistake in res_strings string array

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/atm/firestream.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/atm/firestream.c b/drivers/atm/firestream.c
index d97c05690faa..4e46dc9e41ad 100644
--- a/drivers/atm/firestream.c
+++ b/drivers/atm/firestream.c
@@ -191,7 +191,7 @@ static char *res_strings[] = {
 	"reserved 37",
 	"reserved 38",
 	"reserved 39",
-	"reseverd 40",
+	"reserved 40",
 	"reserved 41", 
 	"reserved 42", 
 	"reserved 43", 
-- 
2.17.0

^ permalink raw reply related

* Re: [RFC PATCH 1/3] arcnet: com20020: Add memory map of com20020
From: Tobin C. Harding @ 2018-05-08 21:39 UTC (permalink / raw)
  To: Andrea Greco
  Cc: m.grzeschik, Andrea Greco, Rob Herring, Mark Rutland, netdev,
	devicetree, linux-kernel
In-Reply-To: <710123d5-c012-5d8f-1c9c-d197f341e702@gmail.com>

On Tue, May 08, 2018 at 11:36:51AM +0200, Andrea Greco wrote:
> On 05/07/2018 04:55 AM, Tobin C. Harding wrote:
> >On Sat, May 05, 2018 at 11:34:45PM +0200, Andrea Greco wrote:
> >>From: Andrea Greco <a.greco@4sigma.it>
> >
> >Hi Andrea,
> >
> >Here are some (mostly stylistic) suggestions to help you get your driver merged.
> >
> >>Add support for com20022I/com20020, memory mapped chip version.
> >>Support bus: Intel 80xx and Motorola 68xx.
> >>Bus size: Only 8 bit bus size is supported.
> >>Added related device tree bindings
> >>
> >>Signed-off-by: Andrea Greco <a.greco@4sigma.it>
> >>---
> >>  .../devicetree/bindings/net/smsc-com20020.txt      |  23 +++
> >>  drivers/net/arcnet/Kconfig                         |  12 +-
> >>  drivers/net/arcnet/Makefile                        |   1 +
> >>  drivers/net/arcnet/arcdevice.h                     |  27 ++-
> >>  drivers/net/arcnet/com20020-membus.c               | 191 +++++++++++++++++++++
> >>  drivers/net/arcnet/com20020.c                      |   9 +-
> >>  6 files changed, 253 insertions(+), 10 deletions(-)
> >>  create mode 100644 Documentation/devicetree/bindings/net/smsc-com20020.txt
> >>  create mode 100644 drivers/net/arcnet/com20020-membus.c
> >>
> >>diff --git a/Documentation/devicetree/bindings/net/smsc-com20020.txt b/Documentation/devicetree/bindings/net/smsc-com20020.txt
> >>new file mode 100644
> >>index 000000000000..39c5b19c55af
> >>--- /dev/null
> >>+++ b/Documentation/devicetree/bindings/net/smsc-com20020.txt
> >>@@ -0,0 +1,23 @@
> >>+SMSC com20020, com20022I
> >>+
> >>+timeout: Arcnet timeout, checkout datashet
> >>+clockp: Clock Prescaler, checkout datashet
> >>+clockm: Clock multiplier, checkout datasheet
> >>+
> >>+phy-reset-gpios: Chip reset ppin
> >>+phy-irq-gpios: Chip irq pin
> 
> ppin Corrected by my-self.
> 
> >>+
> >>+com20020_A@0 {
> >>+    compatible = "smsc,com20020";
> >>+
> >>+	timeout	= <0x3>;
> >>+	backplane = <0x0>;
> >>+
> >>+	clockp = <0x0>;
> >>+	clockm = <0x3>;
> >>+
> >>+	phy-reset-gpios = <&gpio3 21 GPIO_ACTIVE_LOW>;
> >>+	phy-irq-gpios = <&gpio2 10 GPIO_ACTIVE_LOW>;
> >>+
> >>+	status = "okay";
> >>+};
> >>diff --git a/drivers/net/arcnet/Kconfig b/drivers/net/arcnet/Kconfig
> >>index 39bd16f3f86d..d39faf45be1e 100644
> >>--- a/drivers/net/arcnet/Kconfig
> >>+++ b/drivers/net/arcnet/Kconfig
> >>@@ -3,7 +3,7 @@
> >>  #
> >>  menuconfig ARCNET
> >>-	depends on NETDEVICES && (ISA || PCI || PCMCIA)
> >>+	depends on NETDEVICES
> >>  	tristate "ARCnet support"
> >>  	---help---
> >>  	  If you have a network card of this type, say Y and check out the
> >>@@ -129,5 +129,15 @@ config ARCNET_COM20020_CS
> >>  	  To compile this driver as a module, choose M here: the module will be
> >>  	  called com20020_cs.  If unsure, say N.
> >>+config ARCNET_COM20020_MEMORY_BUS
> >>+	bool "Support for COM20020 on external memory"
> >>+	depends on ARCNET_COM20020 && !(ARCNET_COM20020_PCI || ARCNET_COM20020_ISA || ARCNET_COM20020_CS)
> >>+	help
> >>+	  Say Y here if on your custom board mount com20020 or friends.
> >>+
> >>+	  Com20022I support arcnet bus 10Mbitps.
> >>+	  This driver support only 8bit
> >
> >This driver only supports 8bit bus size.
> >
> >>         	    	 	       , and DMA is not supported is attached on your board at external interface bus.
> >
> >This bit does not make sense, sorry.
> Removed description,
> 
> Proposal for v2:
> Say Y here if your custom board mount com20020 chipset or friends.
> Supported Chipset: com20020, com20022, com20022I-3v3.
> If unsure, say N.

If you don't mind me doing review I'll do so again on v2 and comment then.

> >>+	  Supported bus Intel80xx / Motorola 68xx.
> >>+	  This driver not work with other com20020 module: PCI or PCMCIA compiled as [M].
> >
> >I'm not sure exactly what you want to say here, perhaps:
> >
> >   	  This driver does not work with other com20020 modules compiled
> >	  as PCI or PCMCIA [M].
> 
> About this, all removed from kconfig
> For PCI, PCMCIA, checkout downside
> 
> >>  endif # ARCNET
> >>diff --git a/drivers/net/arcnet/Makefile b/drivers/net/arcnet/Makefile
> >>index 53525e8ea130..19425c1e06f4 100644
> >>--- a/drivers/net/arcnet/Makefile
> >>+++ b/drivers/net/arcnet/Makefile
> >>@@ -14,3 +14,4 @@ obj-$(CONFIG_ARCNET_COM20020) += com20020.o
> >>  obj-$(CONFIG_ARCNET_COM20020_ISA) += com20020-isa.o
> >>  obj-$(CONFIG_ARCNET_COM20020_PCI) += com20020-pci.o
> >>  obj-$(CONFIG_ARCNET_COM20020_CS) += com20020_cs.o
> >>+obj-$(CONFIG_ARCNET_COM20020_MEMORY_BUS) += com20020-membus.o
> >>diff --git a/drivers/net/arcnet/arcdevice.h b/drivers/net/arcnet/arcdevice.h
> >>index d09b2b46ab63..16c608269cca 100644
> >>--- a/drivers/net/arcnet/arcdevice.h
> >>+++ b/drivers/net/arcnet/arcdevice.h
> >>@@ -201,7 +201,7 @@ struct ArcProto {
> >>  	void (*rx)(struct net_device *dev, int bufnum,
> >>  		   struct archdr *pkthdr, int length);
> >>  	int (*build_header)(struct sk_buff *skb, struct net_device *dev,
> >>-			    unsigned short ethproto, uint8_t daddr);
> >>+				unsigned short ethproto, uint8_t daddr);
> >
> >   +			    unsigned short ethproto, uint8_t daddr);
> >
> >Please use Linux coding style style, parameters continuing on separate
> >line are aligned with opening parenthesis.
> >
> >>  	/* these functions return '1' if the skb can now be freed */
> >>  	int (*prepare_tx)(struct net_device *dev, struct archdr *pkt,
> >>@@ -326,9 +326,9 @@ struct arcnet_local {
> >>  		void (*recontrigger) (struct net_device * dev, int enable);
> >>  		void (*copy_to_card)(struct net_device *dev, int bufnum,
> >>-				     int offset, void *buf, int count);
> >>+					 int offset, void *buf, int count);
> >>  		void (*copy_from_card)(struct net_device *dev, int bufnum,
> >>-				       int offset, void *buf, int count);
> >>+					   int offset, void *buf, int count);
> >>  	} hw;
> >>  	void __iomem *mem_start;	/* pointer to ioremap'ed MMIO */
> >>@@ -360,7 +360,7 @@ struct net_device *alloc_arcdev(const char *name);
> >>  int arcnet_open(struct net_device *dev);
> >>  int arcnet_close(struct net_device *dev);
> >>  netdev_tx_t arcnet_send_packet(struct sk_buff *skb,
> >>-			       struct net_device *dev);
> >>+				   struct net_device *dev);
> >>  void arcnet_timeout(struct net_device *dev);
> 
> Not required modification then all removed.
> 
> >>  /* I/O equivalents */
> >>@@ -371,7 +371,23 @@ void arcnet_timeout(struct net_device *dev);
> >>  #define BUS_ALIGN  1
> >>  #endif
> >>-/* addr and offset allow register like names to define the actual IO  address.
> >>+#ifdef CONFIG_ARCNET_COM20020_MEMORY_BUS
> >>+#define arcnet_inb(addr, offset)					\
> >>+	ioread8((void __iomem *)(addr) + BUS_ALIGN * (offset))
> >>+
> >>+#define arcnet_outb(value, addr, offset)				\
> >>+	iowrite8(value, (void __iomem *)(addr) + BUS_ALIGN * (offset))
> >>+
> >>+#define arcnet_insb(addr, offset, buffer, count)			\
> >>+	ioread8_rep((void __iomem *)					\
> >>+	(addr) + BUS_ALIGN * (offset), buffer, count)
> >>+
> >>+#define arcnet_outsb(addr, offset, buffer, count)			\
> >>+	iowrite8_rep((void __iomem *)					\
> >>+	(addr) + BUS_ALIGN * (offset), buffer, count)
> >>+#else
> >>+/**
> >>+ * addr and offset allow register like names to define the actual IO  address.
> >>   * A configuration option multiplies the offset for alignment.
> >>   */
> >>  #define arcnet_inb(addr, offset)					\
> >>@@ -388,6 +404,7 @@ void arcnet_timeout(struct net_device *dev);
> >>  	readb((addr) + (offset))
> >>  #define arcnet_writeb(value, addr, offset)				\
> >>  	writeb(value, (addr) + (offset))
> >>+#endif
> >>  #endif				/* __KERNEL__ */
> >>  #endif				/* _LINUX_ARCDEVICE_H */
> 
> 
> This is most important part where a suggestion will be very appreciated.
> This part define how arcnet drivers read and write over physical.
> The old driver can always use readb/writeb and friends, this driver rise big
> kernel panic.
> 
> This driver make a ioreamp with: devm_ioremap.
> Then, for r/w operation i use ioread8/iowrite8 and friends.
> 
> My quickly solution was make a ugly #ifdef.
> 
> With #ifndef all other driver implementation could not be used together
> this driver, because break, how driver write over physical.
> A proposal could be add a read/write callback to arcdevice.h struct hw.
> 
> PROS:
> Every driver fill this callback, this is a solution.
> 
> CONS:
> This solution require a small change for all com20020 driver
> implementations. I don't dispose of all hardware for make a accurate
> test. I could only test memory bus version.
> 
> Any other ideas, will be very appreciated.

I had a bit of a think about this for you.  Can I start by saying I am
not an overly experienced driver developer (or kernel developer for that
matter) so please take all suggestions with this in mind.

Instead of using ifdef's to define arnet_inb/outb() what if you define a
set of functions arnet_readb(), arnet_writeb() etc and use them after
you have done the memory map.  (I had to look up the difference between
readb() and inb() so if this suggestion is complete garbage please
ignore me.)

Also while reading com20020-membus.c I saw a few more changes that you
can use if you so please.  Here is a patch with comments added

diff --git a/drivers/net/arcnet/com20020-membus.c b/drivers/net/arcnet/com20020-membus.c
index 9eead734a3cf..9f3d9b8d9d1f 100644
--- a/drivers/net/arcnet/com20020-membus.c
+++ b/drivers/net/arcnet/com20020-membus.c
@@ -39,7 +39,7 @@ static int com20020_probe(struct platform_device *pdev)
        struct net_device *dev;
        struct arcnet_local *lp;
        struct resource res, *iores;
-       int ret, phy_reset, err;
+       int ret, phy_reset;

This function uses 'ret', 'err', and 'phy_reset' for return value.  I
see what you are getting at by using 'phy_reset' but I don't see the
value in having both 'err' and 'ret' in the same function.

        u32 timeout, backplane, clockp, clockm;
        void __iomem *ioaddr;
 
@@ -47,8 +47,9 @@ static int com20020_probe(struct platform_device *pdev)
 
        iores = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 
-       if (of_address_to_resource(np, 0, &res))
-               return -EINVAL;
+       ret = of_address_to_resource(np, 0, &res);
+       if (ret)
+               return ret;
 
of_address_to_resource() returns -EINVAL on error so we can return it
and maintain program logic.  I see other places intree however that
call of_address_to_resource() as you have done.

        ret = of_property_read_u32(np, "timeout", &timeout);
        if (ret) {
@@ -77,16 +78,17 @@ static int com20020_probe(struct platform_device *pdev)
        phy_reset = of_get_named_gpio(np, "phy-reset-gpios", 0);
        if (phy_reset == -EPROBE_DEFER) {
                return phy_reset;
-       } else if (!gpio_is_valid(phy_reset)) {
+       }

No need for else statement after 'return' (golang linter complains about
this so I always notice it :)

+       if (!gpio_is_valid(phy_reset)) {
                dev_err(&pdev->dev, "phy-reset-gpios not valid !");
-               return 0;
+               return 0;       /* why return 0 after calling dev_err() */

(Added code comment so it would show up in patch.)  I couldn't
understand why this returns 0 here.  Is it what you meant to do?

        }
 
-       err = devm_gpio_request_one(&pdev->dev, phy_reset, GPIOF_OUT_INIT_LOW,
+       ret = devm_gpio_request_one(&pdev->dev, phy_reset, GPIOF_OUT_INIT_LOW,
                                    "arcnet-phy-reset");
-       if (err) {
-               dev_err(&pdev->dev, "failed to get phy-reset-gpios: %d\n", err);
-               return err;

Already commented on above.


Hope this helps,
Tobin.

^ permalink raw reply related

* ICMP redirect and VRF
From: Ben Greear @ 2018-05-08 21:27 UTC (permalink / raw)
  To: netdev

While debugging some other problem today on a system using ip rules instead of
VRF, I ran into a case where the remote router was sending back ICMP redirects.

That got me thinking...where would these routes get stored in a VRF scenario?

Would it magically go to the correct VRF routing table based on the incoming interface
for the ICMP redirect response?

Thanks,
Ben
-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

^ permalink raw reply

* Re: [PATCH bpf-next 5/6] bpf: btf: Update tools/include/uapi/linux/btf.h with BTF ID
From: Song Liu @ 2018-05-08 21:18 UTC (permalink / raw)
  To: Martin Lau
  Cc: netdev@vger.kernel.org, Alexei Starovoitov, Daniel Borkmann,
	Kernel Team
In-Reply-To: <20180504214955.1058805-6-kafai@fb.com>



> On May 4, 2018, at 2:49 PM, Martin KaFai Lau <kafai@fb.com> wrote:
> 
> This patch sync the tools/include/uapi/linux/btf.h with
> the newly introduced BTF ID support.
> 
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> Acked-by: Alexei Starovoitov <ast@fb.com>
> ---
> tools/include/uapi/linux/bpf.h | 11 +++++++++++
> 1 file changed, 11 insertions(+)
> 
> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> index 83a95ae388dd..fff51c187d1e 100644
> --- a/tools/include/uapi/linux/bpf.h
> +++ b/tools/include/uapi/linux/bpf.h
> @@ -96,6 +96,7 @@ enum bpf_cmd {
> 	BPF_PROG_QUERY,
> 	BPF_RAW_TRACEPOINT_OPEN,
> 	BPF_BTF_LOAD,
> +	BPF_BTF_GET_FD_BY_ID,
> };
> 
> enum bpf_map_type {
> @@ -343,6 +344,7 @@ union bpf_attr {
> 			__u32		start_id;
> 			__u32		prog_id;
> 			__u32		map_id;
> +			__u32		btf_id;
> 		};
> 		__u32		next_id;
> 		__u32		open_flags;
> @@ -2129,6 +2131,15 @@ struct bpf_map_info {
> 	__u32 ifindex;
> 	__u64 netns_dev;
> 	__u64 netns_ino;
> +	__u32 btf_id;
> +	__u32 btf_key_id;
> +	__u32 btf_value_id;
> +} __attribute__((aligned(8)));
> +
> +struct bpf_btf_info {
> +	__aligned_u64 btf;
> +	__u32 btf_size;
> +	__u32 id;
> } __attribute__((aligned(8)));
> 
> /* User bpf_sock_addr struct to access socket fields and sockaddr struct passed
> -- 
> 2.9.5
> 

Acked-by: Song Liu <songliubraving@fb.com>

^ permalink raw reply

* Re: [PATCH bpf-next 6/6] bpf: btf: Tests for BPF_OBJ_GET_INFO_BY_FD and BPF_BTF_GET_FD_BY_ID
From: Song Liu @ 2018-05-08 21:18 UTC (permalink / raw)
  To: Martin Lau
  Cc: netdev@vger.kernel.org, Alexei Starovoitov, Daniel Borkmann,
	Kernel Team
In-Reply-To: <20180504214955.1058805-7-kafai@fb.com>



> On May 4, 2018, at 2:49 PM, Martin KaFai Lau <kafai@fb.com> wrote:
> 
> This patch adds test for BPF_BTF_GET_FD_BY_ID and the new
> btf_id/btf_key_id/btf_value_id in the "struct bpf_map_info".
> 
> It also modifies the existing BPF_OBJ_GET_INFO_BY_FD test
> to reflect the new "struct bpf_btf_info".
> 
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> Acked-by: Alexei Starovoitov <ast@fb.com>
> ---
> tools/lib/bpf/bpf.c                    |  10 ++
> tools/lib/bpf/bpf.h                    |   1 +
> tools/testing/selftests/bpf/test_btf.c | 289 +++++++++++++++++++++++++++++++--
> 3 files changed, 287 insertions(+), 13 deletions(-)
> 
> diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
> index 76b36cc16e7f..a3a8fb2ac697 100644
> --- a/tools/lib/bpf/bpf.c
> +++ b/tools/lib/bpf/bpf.c
> @@ -458,6 +458,16 @@ int bpf_map_get_fd_by_id(__u32 id)
> 	return sys_bpf(BPF_MAP_GET_FD_BY_ID, &attr, sizeof(attr));
> }
> 
> +int bpf_btf_get_fd_by_id(__u32 id)
> +{
> +	union bpf_attr attr;
> +
> +	bzero(&attr, sizeof(attr));
> +	attr.btf_id = id;
> +
> +	return sys_bpf(BPF_BTF_GET_FD_BY_ID, &attr, sizeof(attr));
> +}
> +
> int bpf_obj_get_info_by_fd(int prog_fd, void *info, __u32 *info_len)
> {
> 	union bpf_attr attr;
> diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
> index 553b11ad52b3..fb3a146d92ff 100644
> --- a/tools/lib/bpf/bpf.h
> +++ b/tools/lib/bpf/bpf.h
> @@ -98,6 +98,7 @@ int bpf_prog_get_next_id(__u32 start_id, __u32 *next_id);
> int bpf_map_get_next_id(__u32 start_id, __u32 *next_id);
> int bpf_prog_get_fd_by_id(__u32 id);
> int bpf_map_get_fd_by_id(__u32 id);
> +int bpf_btf_get_fd_by_id(__u32 id);
> int bpf_obj_get_info_by_fd(int prog_fd, void *info, __u32 *info_len);
> int bpf_prog_query(int target_fd, enum bpf_attach_type type, __u32 query_flags,
> 		   __u32 *attach_flags, __u32 *prog_ids, __u32 *prog_cnt);
> diff --git a/tools/testing/selftests/bpf/test_btf.c b/tools/testing/selftests/bpf/test_btf.c
> index b7880a20fad1..c8bceae7ec02 100644
> --- a/tools/testing/selftests/bpf/test_btf.c
> +++ b/tools/testing/selftests/bpf/test_btf.c
> @@ -1047,9 +1047,13 @@ struct btf_get_info_test {
> 	const char *str_sec;
> 	__u32 raw_types[MAX_NR_RAW_TYPES];
> 	__u32 str_sec_size;
> -	int info_size_delta;
> +	int btf_size_delta;
> +	int (*special_test)(unsigned int test_num);
> };
> 
> +static int test_big_btf_info(unsigned int test_num);
> +static int test_btf_id(unsigned int test_num);
> +
> const struct btf_get_info_test get_info_tests[] = {
> {
> 	.descr = "== raw_btf_size+1",
> @@ -1060,7 +1064,7 @@ const struct btf_get_info_test get_info_tests[] = {
> 	},
> 	.str_sec = "",
> 	.str_sec_size = sizeof(""),
> -	.info_size_delta = 1,
> +	.btf_size_delta = 1,
> },
> {
> 	.descr = "== raw_btf_size-3",
> @@ -1071,20 +1075,274 @@ const struct btf_get_info_test get_info_tests[] = {
> 	},
> 	.str_sec = "",
> 	.str_sec_size = sizeof(""),
> -	.info_size_delta = -3,
> +	.btf_size_delta = -3,
> +},
> +{
> +	.descr = "Large bpf_btf_info",
> +	.raw_types = {
> +		/* int */				/* [1] */
> +		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),
> +		BTF_END_RAW,
> +	},
> +	.str_sec = "",
> +	.str_sec_size = sizeof(""),
> +	.special_test = test_big_btf_info,
> +},
> +{
> +	.descr = "BTF ID",
> +	.raw_types = {
> +		/* int */				/* [1] */
> +		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),
> +		/* unsigned int */			/* [2] */
> +		BTF_TYPE_INT_ENC(0, 0, 0, 32, 4),
> +		BTF_END_RAW,
> +	},
> +	.str_sec = "",
> +	.str_sec_size = sizeof(""),
> +	.special_test = test_btf_id,
> },
> };
> 
> +static inline __u64 ptr_to_u64(const void *ptr)
> +{
> +	return (__u64)(unsigned long)ptr;
> +}
> +
> +static int test_big_btf_info(unsigned int test_num)
> +{
> +	const struct btf_get_info_test *test = &get_info_tests[test_num - 1];
> +	uint8_t *raw_btf = NULL, *user_btf = NULL;
> +	unsigned int raw_btf_size;
> +	struct {
> +		struct bpf_btf_info info;
> +		uint64_t garbage;
> +	} info_garbage;
> +	struct bpf_btf_info *info;
> +	int btf_fd = -1, err;
> +	uint32_t info_len;
> +
> +	raw_btf = btf_raw_create(&hdr_tmpl,
> +				 test->raw_types,
> +				 test->str_sec,
> +				 test->str_sec_size,
> +				 &raw_btf_size);
> +
> +	if (!raw_btf)
> +		return -1;
> +
> +	*btf_log_buf = '\0';
> +
> +	user_btf = malloc(raw_btf_size);
> +	if (CHECK(!user_btf, "!user_btf")) {
> +		err = -1;
> +		goto done;
> +	}
> +
> +	btf_fd = bpf_load_btf(raw_btf, raw_btf_size,
> +			      btf_log_buf, BTF_LOG_BUF_SIZE,
> +			      args.always_log);
> +	if (CHECK(btf_fd == -1, "errno:%d", errno)) {
> +		err = -1;
> +		goto done;
> +	}
> +
> +	/*
> +	 * GET_INFO should error out if the userspace info
> +	 * has non zero tailing bytes.
> +	 */
> +	info = &info_garbage.info;
> +	memset(info, 0, sizeof(*info));
> +	info_garbage.garbage = 0xdeadbeef;
> +	info_len = sizeof(info_garbage);
> +	info->btf = ptr_to_u64(user_btf);
> +	info->btf_size = raw_btf_size;
> +
> +	err = bpf_obj_get_info_by_fd(btf_fd, info, &info_len);
> +	if (CHECK(!err, "!err")) {
> +		err = -1;
> +		goto done;
> +	}
> +
> +	/*
> +	 * GET_INFO should succeed even info_len is larger than
> +	 * the kernel supported as long as tailing bytes are zero.
> +	 * The kernel supported info len should also be returned
> +	 * to userspace.
> +	 */
> +	info_garbage.garbage = 0;
> +	err = bpf_obj_get_info_by_fd(btf_fd, info, &info_len);
> +	if (CHECK(err || info_len != sizeof(*info),
> +		  "err:%d errno:%d info_len:%u sizeof(*info):%lu",
> +		  err, errno, info_len, sizeof(*info))) {
> +		err = -1;
> +		goto done;
> +	}
> +
> +	fprintf(stderr, "OK");
> +
> +done:
> +	if (*btf_log_buf && (err || args.always_log))
> +		fprintf(stderr, "\n%s", btf_log_buf);
> +
> +	free(raw_btf);
> +	free(user_btf);
> +
> +	if (btf_fd != -1)
> +		close(btf_fd);
> +
> +	return err;
> +}
> +
> +static int test_btf_id(unsigned int test_num)
> +{
> +	const struct btf_get_info_test *test = &get_info_tests[test_num - 1];
> +	struct bpf_create_map_attr create_attr = {};
> +	uint8_t *raw_btf = NULL, *user_btf[2] = {};
> +	int btf_fd[2] = {-1, -1}, map_fd = -1;
> +	struct bpf_map_info map_info = {};
> +	struct bpf_btf_info info[2] = {};
> +	unsigned int raw_btf_size;
> +	uint32_t info_len;
> +	int err, i, ret;
> +
> +	raw_btf = btf_raw_create(&hdr_tmpl,
> +				 test->raw_types,
> +				 test->str_sec,
> +				 test->str_sec_size,
> +				 &raw_btf_size);
> +
> +	if (!raw_btf)
> +		return -1;
> +
> +	*btf_log_buf = '\0';
> +
> +	for (i = 0; i < 2; i++) {
> +		user_btf[i] = malloc(raw_btf_size);
> +		if (CHECK(!user_btf[i], "!user_btf[%d]", i)) {
> +			err = -1;
> +			goto done;
> +		}
> +		info[i].btf = ptr_to_u64(user_btf[i]);
> +		info[i].btf_size = raw_btf_size;
> +	}
> +
> +	btf_fd[0] = bpf_load_btf(raw_btf, raw_btf_size,
> +				 btf_log_buf, BTF_LOG_BUF_SIZE,
> +				 args.always_log);
> +	if (CHECK(btf_fd[0] == -1, "errno:%d", errno)) {
> +		err = -1;
> +		goto done;
> +	}
> +
> +	/* Test BPF_OBJ_GET_INFO_BY_ID on btf_id */
> +	info_len = sizeof(info[0]);
> +	err = bpf_obj_get_info_by_fd(btf_fd[0], &info[0], &info_len);
> +	if (CHECK(err, "errno:%d", errno)) {
> +		err = -1;
> +		goto done;
> +	}
> +
> +	btf_fd[1] = bpf_btf_get_fd_by_id(info[0].id);
> +	if (CHECK(btf_fd[1] == -1, "errno:%d", errno)) {
> +		err = -1;
> +		goto done;
> +	}
> +
> +	ret = 0;
> +	err = bpf_obj_get_info_by_fd(btf_fd[1], &info[1], &info_len);
> +	if (CHECK(err || info[0].id != info[1].id ||
> +		  info[0].btf_size != info[1].btf_size ||
> +		  (ret = memcmp(user_btf[0], user_btf[1], info[0].btf_size)),
> +		  "err:%d errno:%d id0:%u id1:%u btf_size0:%u btf_size1:%u memcmp:%d",
> +		  err, errno, info[0].id, info[1].id,
> +		  info[0].btf_size, info[1].btf_size, ret)) {
> +		err = -1;
> +		goto done;
> +	}
> +
> +	/* Test btf members in struct bpf_map_info */
> +	create_attr.name = "test_btf_id";
> +	create_attr.map_type = BPF_MAP_TYPE_ARRAY;
> +	create_attr.key_size = sizeof(int);
> +	create_attr.value_size = sizeof(unsigned int);
> +	create_attr.max_entries = 4;
> +	create_attr.btf_fd = btf_fd[0];
> +	create_attr.btf_key_id = 1;
> +	create_attr.btf_value_id = 2;
> +
> +	map_fd = bpf_create_map_xattr(&create_attr);
> +	if (CHECK(map_fd == -1, "errno:%d", errno)) {
> +		err = -1;
> +		goto done;
> +	}
> +
> +	info_len = sizeof(map_info);
> +	err = bpf_obj_get_info_by_fd(map_fd, &map_info, &info_len);
> +	if (CHECK(err || map_info.btf_id != info[0].id ||
> +		  map_info.btf_key_id != 1 || map_info.btf_value_id != 2,
> +		  "err:%d errno:%d info.id:%u btf_id:%u btf_key_id:%u btf_value_id:%u",
> +		  err, errno, info[0].id, map_info.btf_id, map_info.btf_key_id,
> +		  map_info.btf_value_id)) {
> +		err = -1;
> +		goto done;
> +	}
> +
> +	for (i = 0; i < 2; i++) {
> +		close(btf_fd[i]);
> +		btf_fd[i] = -1;
> +	}
> +
> +	/* Test BTF ID is removed from the kernel */
> +	btf_fd[0] = bpf_btf_get_fd_by_id(map_info.btf_id);
> +	if (CHECK(btf_fd[0] == -1, "errno:%d", errno)) {
> +		err = -1;
> +		goto done;
> +	}
> +	close(btf_fd[0]);
> +	btf_fd[0] = -1;
> +
> +	/* The map holds the last ref to BTF and its btf_id */
> +	close(map_fd);
> +	map_fd = -1;
> +	btf_fd[0] = bpf_btf_get_fd_by_id(map_info.btf_id);
> +	if (CHECK(btf_fd[0] != -1, "BTF lingers")) {
> +		err = -1;
> +		goto done;
> +	}
> +
> +	fprintf(stderr, "OK");
> +
> +done:
> +	if (*btf_log_buf && (err || args.always_log))
> +		fprintf(stderr, "\n%s", btf_log_buf);
> +
> +	free(raw_btf);
> +	if (map_fd != -1)
> +		close(map_fd);
> +	for (i = 0; i < 2; i++) {
> +		free(user_btf[i]);
> +		if (btf_fd[i] != -1)
> +			close(btf_fd[i]);
> +	}
> +
> +	return err;
> +}
> +
> static int do_test_get_info(unsigned int test_num)
> {
> 	const struct btf_get_info_test *test = &get_info_tests[test_num - 1];
> 	unsigned int raw_btf_size, user_btf_size, expected_nbytes;
> 	uint8_t *raw_btf = NULL, *user_btf = NULL;
> -	int btf_fd = -1, err;
> +	struct bpf_btf_info info = {};
> +	int btf_fd = -1, err, ret;
> +	uint32_t info_len;
> 
> -	fprintf(stderr, "BTF GET_INFO_BY_ID test[%u] (%s): ",
> +	fprintf(stderr, "BTF GET_INFO test[%u] (%s): ",
> 		test_num, test->descr);
> 
> +	if (test->special_test)
> +		return test->special_test(test_num);
> +
> 	raw_btf = btf_raw_create(&hdr_tmpl,
> 				 test->raw_types,
> 				 test->str_sec,
> @@ -1110,19 +1368,24 @@ static int do_test_get_info(unsigned int test_num)
> 		goto done;
> 	}
> 
> -	user_btf_size = (int)raw_btf_size + test->info_size_delta;
> +	user_btf_size = (int)raw_btf_size + test->btf_size_delta;
> 	expected_nbytes = min(raw_btf_size, user_btf_size);
> 	if (raw_btf_size > expected_nbytes)
> 		memset(user_btf + expected_nbytes, 0xff,
> 		       raw_btf_size - expected_nbytes);
> 
> -	err = bpf_obj_get_info_by_fd(btf_fd, user_btf, &user_btf_size);
> -	if (CHECK(err || user_btf_size != raw_btf_size ||
> -		  memcmp(raw_btf, user_btf, expected_nbytes),
> -		  "err:%d(errno:%d) raw_btf_size:%u user_btf_size:%u expected_nbytes:%u memcmp:%d",
> -		  err, errno,
> -		  raw_btf_size, user_btf_size, expected_nbytes,
> -		  memcmp(raw_btf, user_btf, expected_nbytes))) {
> +	info_len = sizeof(info);
> +	info.btf = ptr_to_u64(user_btf);
> +	info.btf_size = user_btf_size;
> +
> +	ret = 0;
> +	err = bpf_obj_get_info_by_fd(btf_fd, &info, &info_len);
> +	if (CHECK(err || !info.id || info_len != sizeof(info) ||
> +		  info.btf_size != raw_btf_size ||
> +		  (ret = memcmp(raw_btf, user_btf, expected_nbytes)),
> +		  "err:%d errno:%d info.id:%u info_len:%u sizeof(info):%lu raw_btf_size:%u info.btf_size:%u expected_nbytes:%u memcmp:%d",
> +		  err, errno, info.id, info_len, sizeof(info),
> +		  raw_btf_size, info.btf_size, expected_nbytes, ret)) {
> 		err = -1;
> 		goto done;
> 	}
> -- 
> 2.9.5
> 

Acked-by: Song Liu <songliubraving@fb.com>

^ permalink raw reply

* Re: [PATCH bpf-next 4/6] bpf: btf: Some test_btf clean up
From: Song Liu @ 2018-05-08 21:18 UTC (permalink / raw)
  To: Martin Lau
  Cc: netdev@vger.kernel.org, Alexei Starovoitov, Daniel Borkmann,
	Kernel Team
In-Reply-To: <20180504214955.1058805-5-kafai@fb.com>



> On May 4, 2018, at 2:49 PM, Martin KaFai Lau <kafai@fb.com> wrote:
> 
> This patch adds a CHECK() macro for condition checking
> and error report purpose.  Something similar to test_progs.c
> 
> It also counts the number of tests passed/skipped/failed and
> print them at the end of the test run.
> 
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> Acked-by: Alexei Starovoitov <ast@fb.com>
> ---
> tools/testing/selftests/bpf/test_btf.c | 201 ++++++++++++++++-----------------
> 1 file changed, 99 insertions(+), 102 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/test_btf.c b/tools/testing/selftests/bpf/test_btf.c
> index 7b39b1f712a1..b7880a20fad1 100644
> --- a/tools/testing/selftests/bpf/test_btf.c
> +++ b/tools/testing/selftests/bpf/test_btf.c
> @@ -20,6 +20,30 @@
> 
> #include "bpf_rlimit.h"
> 
> +static uint32_t pass_cnt;
> +static uint32_t error_cnt;
> +static uint32_t skip_cnt;
> +
> +#define CHECK(condition, format...) ({					\
> +	int __ret = !!(condition);					\
> +	if (__ret) {							\
> +		fprintf(stderr, "%s:%d:FAIL ", __func__, __LINE__);	\
> +		fprintf(stderr, format);				\
> +	}								\
> +	__ret;								\
> +})
> +
> +static int count_result(int err)
> +{
> +	if (err)
> +		error_cnt++;
> +	else
> +		pass_cnt++;
> +
> +	fprintf(stderr, "\n");
> +	return err;
> +}
> +
> #define min(a, b) ((a) < (b) ? (a) : (b))
> #define __printf(a, b)	__attribute__((format(printf, a, b)))
> 
> @@ -894,17 +918,13 @@ static void *btf_raw_create(const struct btf_header *hdr,
> 	void *raw_btf;
> 
> 	type_sec_size = get_type_sec_size(raw_types);
> -	if (type_sec_size < 0) {
> -		fprintf(stderr, "Cannot get nr_raw_types\n");
> +	if (CHECK(type_sec_size < 0, "Cannot get nr_raw_types"))
> 		return NULL;
> -	}
> 
> 	size_needed = sizeof(*hdr) + type_sec_size + str_sec_size;
> 	raw_btf = malloc(size_needed);
> -	if (!raw_btf) {
> -		fprintf(stderr, "Cannot allocate memory for raw_btf\n");
> +	if (CHECK(!raw_btf, "Cannot allocate memory for raw_btf"))
> 		return NULL;
> -	}
> 
> 	/* Copy header */
> 	memcpy(raw_btf, hdr, sizeof(*hdr));
> @@ -915,8 +935,7 @@ static void *btf_raw_create(const struct btf_header *hdr,
> 	for (i = 0; i < type_sec_size / sizeof(raw_types[0]); i++) {
> 		if (raw_types[i] == NAME_TBD) {
> 			next_str = get_next_str(next_str, end_str);
> -			if (!next_str) {
> -				fprintf(stderr, "Error in getting next_str\n");
> +			if (CHECK(!next_str, "Error in getting next_str")) {
> 				free(raw_btf);
> 				return NULL;
> 			}
> @@ -973,9 +992,8 @@ static int do_test_raw(unsigned int test_num)
> 	free(raw_btf);
> 
> 	err = ((btf_fd == -1) != test->btf_load_err);
> -	if (err)
> -		fprintf(stderr, "btf_load_err:%d btf_fd:%d\n",
> -			test->btf_load_err, btf_fd);
> +	CHECK(err, "btf_fd:%d test->btf_load_err:%u",
> +	      btf_fd, test->btf_load_err);
> 
> 	if (err || btf_fd == -1)
> 		goto done;
> @@ -992,16 +1010,15 @@ static int do_test_raw(unsigned int test_num)
> 	map_fd = bpf_create_map_xattr(&create_attr);
> 
> 	err = ((map_fd == -1) != test->map_create_err);
> -	if (err)
> -		fprintf(stderr, "map_create_err:%d map_fd:%d\n",
> -			test->map_create_err, map_fd);
> +	CHECK(err, "map_fd:%d test->map_create_err:%u",
> +	      map_fd, test->map_create_err);
> 
> done:
> 	if (!err)
> -		fprintf(stderr, "OK\n");
> +		fprintf(stderr, "OK");
> 
> 	if (*btf_log_buf && (err || args.always_log))
> -		fprintf(stderr, "%s\n", btf_log_buf);
> +		fprintf(stderr, "\n%s", btf_log_buf);
> 
> 	if (btf_fd != -1)
> 		close(btf_fd);
> @@ -1017,10 +1034,10 @@ static int test_raw(void)
> 	int err = 0;
> 
> 	if (args.raw_test_num)
> -		return do_test_raw(args.raw_test_num);
> +		return count_result(do_test_raw(args.raw_test_num));
> 
> 	for (i = 1; i <= ARRAY_SIZE(raw_tests); i++)
> -		err |= do_test_raw(i);
> +		err |= count_result(do_test_raw(i));
> 
> 	return err;
> }
> @@ -1080,8 +1097,7 @@ static int do_test_get_info(unsigned int test_num)
> 	*btf_log_buf = '\0';
> 
> 	user_btf = malloc(raw_btf_size);
> -	if (!user_btf) {
> -		fprintf(stderr, "Cannot allocate memory for user_btf\n");
> +	if (CHECK(!user_btf, "!user_btf")) {
> 		err = -1;
> 		goto done;
> 	}
> @@ -1089,9 +1105,7 @@ static int do_test_get_info(unsigned int test_num)
> 	btf_fd = bpf_load_btf(raw_btf, raw_btf_size,
> 			      btf_log_buf, BTF_LOG_BUF_SIZE,
> 			      args.always_log);
> -	if (btf_fd == -1) {
> -		fprintf(stderr, "bpf_load_btf:%s(%d)\n",
> -			strerror(errno), errno);
> +	if (CHECK(btf_fd == -1, "errno:%d", errno)) {
> 		err = -1;
> 		goto done;
> 	}
> @@ -1103,31 +1117,31 @@ static int do_test_get_info(unsigned int test_num)
> 		       raw_btf_size - expected_nbytes);
> 
> 	err = bpf_obj_get_info_by_fd(btf_fd, user_btf, &user_btf_size);
> -	if (err || user_btf_size != raw_btf_size ||
> -	    memcmp(raw_btf, user_btf, expected_nbytes)) {
> -		fprintf(stderr,
> -			"err:%d(errno:%d) raw_btf_size:%u user_btf_size:%u expected_nbytes:%u memcmp:%d\n",
> -			err, errno,
> -			raw_btf_size, user_btf_size, expected_nbytes,
> -			memcmp(raw_btf, user_btf, expected_nbytes));
> +	if (CHECK(err || user_btf_size != raw_btf_size ||
> +		  memcmp(raw_btf, user_btf, expected_nbytes),
> +		  "err:%d(errno:%d) raw_btf_size:%u user_btf_size:%u expected_nbytes:%u memcmp:%d",
> +		  err, errno,
> +		  raw_btf_size, user_btf_size, expected_nbytes,
> +		  memcmp(raw_btf, user_btf, expected_nbytes))) {
> 		err = -1;
> 		goto done;
> 	}
> 
> 	while (expected_nbytes < raw_btf_size) {
> 		fprintf(stderr, "%u...", expected_nbytes);
> -		if (user_btf[expected_nbytes++] != 0xff) {
> -			fprintf(stderr, "!= 0xff\n");
> +		if (CHECK(user_btf[expected_nbytes++] != 0xff,
> +			  "user_btf[%u]:%x != 0xff", expected_nbytes - 1,
> +			  user_btf[expected_nbytes - 1])) {
> 			err = -1;
> 			goto done;
> 		}
> 	}
> 
> -	fprintf(stderr, "OK\n");
> +	fprintf(stderr, "OK");
> 
> done:
> 	if (*btf_log_buf && (err || args.always_log))
> -		fprintf(stderr, "%s\n", btf_log_buf);
> +		fprintf(stderr, "\n%s", btf_log_buf);
> 
> 	free(raw_btf);
> 	free(user_btf);
> @@ -1144,10 +1158,10 @@ static int test_get_info(void)
> 	int err = 0;
> 
> 	if (args.get_info_test_num)
> -		return do_test_get_info(args.get_info_test_num);
> +		return count_result(do_test_get_info(args.get_info_test_num));
> 
> 	for (i = 1; i <= ARRAY_SIZE(get_info_tests); i++)
> -		err |= do_test_get_info(i);
> +		err |= count_result(do_test_get_info(i));
> 
> 	return err;
> }
> @@ -1175,28 +1189,21 @@ static int file_has_btf_elf(const char *fn)
> 	Elf *elf;
> 	int ret;
> 
> -	if (elf_version(EV_CURRENT) == EV_NONE) {
> -		fprintf(stderr, "Failed to init libelf\n");
> +	if (CHECK(elf_version(EV_CURRENT) == EV_NONE,
> +		  "elf_version(EV_CURRENT) == EV_NONE"))
> 		return -1;
> -	}
> 
> 	elf_fd = open(fn, O_RDONLY);
> -	if (elf_fd == -1) {
> -		fprintf(stderr, "Cannot open file %s: %s(%d)\n",
> -			fn, strerror(errno), errno);
> +	if (CHECK(elf_fd == -1, "open(%s): errno:%d", fn, errno))
> 		return -1;
> -	}
> 
> 	elf = elf_begin(elf_fd, ELF_C_READ, NULL);
> -	if (!elf) {
> -		fprintf(stderr, "Failed to read ELF from %s. %s\n", fn,
> -			elf_errmsg(elf_errno()));
> +	if (CHECK(!elf, "elf_begin(%s): %s", fn, elf_errmsg(elf_errno()))) {
> 		ret = -1;
> 		goto done;
> 	}
> 
> -	if (!gelf_getehdr(elf, &ehdr)) {
> -		fprintf(stderr, "Failed to get EHDR from %s\n", fn);
> +	if (CHECK(!gelf_getehdr(elf, &ehdr), "!gelf_getehdr(%s)", fn)) {
> 		ret = -1;
> 		goto done;
> 	}
> @@ -1205,9 +1212,8 @@ static int file_has_btf_elf(const char *fn)
> 		const char *sh_name;
> 		GElf_Shdr sh;
> 
> -		if (gelf_getshdr(scn, &sh) != &sh) {
> -			fprintf(stderr,
> -				"Failed to get section header from %s\n", fn);
> +		if (CHECK(gelf_getshdr(scn, &sh) != &sh,
> +			  "file:%s gelf_getshdr != &sh", fn)) {
> 			ret = -1;
> 			goto done;
> 		}
> @@ -1243,53 +1249,44 @@ static int do_test_file(unsigned int test_num)
> 		return err;
> 
> 	if (err == 0) {
> -		fprintf(stderr, "SKIP. No ELF %s found\n", BTF_ELF_SEC);
> +		fprintf(stderr, "SKIP. No ELF %s found", BTF_ELF_SEC);
> +		skip_cnt++;
> 		return 0;
> 	}
> 
> 	obj = bpf_object__open(test->file);
> -	if (IS_ERR(obj))
> +	if (CHECK(IS_ERR(obj), "obj: %ld", PTR_ERR(obj)))
> 		return PTR_ERR(obj);
> 
> 	err = bpf_object__btf_fd(obj);
> -	if (err == -1) {
> -		fprintf(stderr, "bpf_object__btf_fd: -1\n");
> +	if (CHECK(err == -1, "bpf_object__btf_fd: -1"))
> 		goto done;
> -	}
> 
> 	prog = bpf_program__next(NULL, obj);
> -	if (!prog) {
> -		fprintf(stderr, "Cannot find bpf_prog\n");
> +	if (CHECK(!prog, "Cannot find bpf_prog")) {
> 		err = -1;
> 		goto done;
> 	}
> 
> 	bpf_program__set_type(prog, BPF_PROG_TYPE_TRACEPOINT);
> 	err = bpf_object__load(obj);
> -	if (err < 0) {
> -		fprintf(stderr, "bpf_object__load: %d\n", err);
> +	if (CHECK(err < 0, "bpf_object__load: %d", err))
> 		goto done;
> -	}
> 
> 	map = bpf_object__find_map_by_name(obj, "btf_map");
> -	if (!map) {
> -		fprintf(stderr, "btf_map not found\n");
> +	if (CHECK(!map, "btf_map not found")) {
> 		err = -1;
> 		goto done;
> 	}
> 
> 	err = (bpf_map__btf_key_id(map) == 0 || bpf_map__btf_value_id(map) == 0)
> 		!= test->btf_kv_notfound;
> -	if (err) {
> -		fprintf(stderr,
> -			"btf_kv_notfound:%u btf_key_id:%u btf_value_id:%u\n",
> -			test->btf_kv_notfound,
> -			bpf_map__btf_key_id(map),
> -			bpf_map__btf_value_id(map));
> +	if (CHECK(err, "btf_key_id:%u btf_value_id:%u test->btf_kv_notfound:%u",
> +		  bpf_map__btf_key_id(map), bpf_map__btf_value_id(map),
> +		  test->btf_kv_notfound))
> 		goto done;
> -	}
> 
> -	fprintf(stderr, "OK\n");
> +	fprintf(stderr, "OK");
> 
> done:
> 	bpf_object__close(obj);
> @@ -1302,10 +1299,10 @@ static int test_file(void)
> 	int err = 0;
> 
> 	if (args.file_test_num)
> -		return do_test_file(args.file_test_num);
> +		return count_result(do_test_file(args.file_test_num));
> 
> 	for (i = 1; i <= ARRAY_SIZE(file_tests); i++)
> -		err |= do_test_file(i);
> +		err |= count_result(do_test_file(i));
> 
> 	return err;
> }
> @@ -1425,7 +1422,7 @@ static int test_pprint(void)
> 	unsigned int key;
> 	uint8_t *raw_btf;
> 	ssize_t nread;
> -	int err;
> +	int err, ret;
> 
> 	fprintf(stderr, "%s......", test->descr);
> 	raw_btf = btf_raw_create(&hdr_tmpl, test->raw_types,
> @@ -1441,10 +1438,8 @@ static int test_pprint(void)
> 			      args.always_log);
> 	free(raw_btf);
> 
> -	if (btf_fd == -1) {
> +	if (CHECK(btf_fd == -1, "errno:%d", errno)) {
> 		err = -1;
> -		fprintf(stderr, "bpf_load_btf: %s(%d)\n",
> -			strerror(errno), errno);
> 		goto done;
> 	}
> 
> @@ -1458,26 +1453,23 @@ static int test_pprint(void)
> 	create_attr.btf_value_id = test->value_id;
> 
> 	map_fd = bpf_create_map_xattr(&create_attr);
> -	if (map_fd == -1) {
> +	if (CHECK(map_fd == -1, "errno:%d", errno)) {
> 		err = -1;
> -		fprintf(stderr, "bpf_creat_map_btf: %s(%d)\n",
> -			strerror(errno), errno);
> 		goto done;
> 	}
> 
> -	if (snprintf(pin_path, sizeof(pin_path), "%s/%s",
> -		     "/sys/fs/bpf", test->map_name) == sizeof(pin_path)) {
> +	ret = snprintf(pin_path, sizeof(pin_path), "%s/%s",
> +		       "/sys/fs/bpf", test->map_name);
> +
> +	if (CHECK(ret == sizeof(pin_path), "pin_path %s/%s is too long",
> +		  "/sys/fs/bpf", test->map_name)) {
> 		err = -1;
> -		fprintf(stderr, "pin_path is too long\n");
> 		goto done;
> 	}
> 
> 	err = bpf_obj_pin(map_fd, pin_path);
> -	if (err) {
> -		fprintf(stderr, "Cannot pin to %s. %s(%d).\n", pin_path,
> -			strerror(errno), errno);
> +	if (CHECK(err, "bpf_obj_pin(%s): errno:%d.", pin_path, errno))
> 		goto done;
> -	}
> 
> 	for (key = 0; key < test->max_entries; key++) {
> 		set_pprint_mapv(&mapv, key);
> @@ -1485,10 +1477,8 @@ static int test_pprint(void)
> 	}
> 
> 	pin_file = fopen(pin_path, "r");
> -	if (!pin_file) {
> +	if (CHECK(!pin_file, "fopen(%s): errno:%d", pin_path, errno)) {
> 		err = -1;
> -		fprintf(stderr, "fopen(%s): %s(%d)\n", pin_path,
> -			strerror(errno), errno);
> 		goto done;
> 	}
> 
> @@ -1497,9 +1487,8 @@ static int test_pprint(void)
> 	       *line == '#')
> 		;
> 
> -	if (nread <= 0) {
> +	if (CHECK(nread <= 0, "Unexpected EOF")) {
> 		err = -1;
> -		fprintf(stderr, "Unexpected EOF\n");
> 		goto done;
> 	}
> 
> @@ -1518,9 +1507,9 @@ static int test_pprint(void)
> 					  mapv.ui8a[4], mapv.ui8a[5], mapv.ui8a[6], mapv.ui8a[7],
> 					  pprint_enum_str[mapv.aenum]);
> 
> -		if (nexpected_line == sizeof(expected_line)) {
> +		if (CHECK(nexpected_line == sizeof(expected_line),
> +			  "expected_line is too long")) {
> 			err = -1;
> -			fprintf(stderr, "expected_line is too long\n");
> 			goto done;
> 		}
> 
> @@ -1535,15 +1524,15 @@ static int test_pprint(void)
> 		nread = getline(&line, &line_len, pin_file);
> 	} while (++key < test->max_entries && nread > 0);
> 
> -	if (key < test->max_entries) {
> +	if (CHECK(key < test->max_entries,
> +		  "Unexpected EOF. key:%u test->max_entries:%u",
> +		  key, test->max_entries)) {
> 		err = -1;
> -		fprintf(stderr, "Unexpected EOF\n");
> 		goto done;
> 	}
> 
> -	if (nread > 0) {
> +	if (CHECK(nread > 0, "Unexpected extra pprint output: %s", line)) {
> 		err = -1;
> -		fprintf(stderr, "Unexpected extra pprint output: %s\n", line);
> 		goto done;
> 	}
> 
> @@ -1551,9 +1540,9 @@ static int test_pprint(void)
> 
> done:
> 	if (!err)
> -		fprintf(stderr, "OK\n");
> +		fprintf(stderr, "OK");
> 	if (*btf_log_buf && (err || args.always_log))
> -		fprintf(stderr, "%s\n", btf_log_buf);
> +		fprintf(stderr, "\n%s", btf_log_buf);
> 	if (btf_fd != -1)
> 		close(btf_fd);
> 	if (map_fd != -1)
> @@ -1634,6 +1623,12 @@ static int parse_args(int argc, char **argv)
> 	return 0;
> }
> 
> +static void print_summary(void)
> +{
> +	fprintf(stderr, "PASS:%u SKIP:%u FAIL:%u\n",
> +		pass_cnt - skip_cnt, skip_cnt, error_cnt);
> +}
> +
> int main(int argc, char **argv)
> {
> 	int err = 0;
> @@ -1655,15 +1650,17 @@ int main(int argc, char **argv)
> 		err |= test_file();
> 
> 	if (args.pprint_test)
> -		err |= test_pprint();
> +		err |= count_result(test_pprint());
> 
> 	if (args.raw_test || args.get_info_test || args.file_test ||
> 	    args.pprint_test)
> -		return err;
> +		goto done;
> 
> 	err |= test_raw();
> 	err |= test_get_info();
> 	err |= test_file();
> 
> +done:
> +	print_summary();
> 	return err;
> }
> -- 
> 2.9.5
> 

Acked-by: Song Liu <songliubraving@fb.com>

^ permalink raw reply

* Re: [PATCH bpf-next 1/6] bpf: btf: Avoid WARN_ON when CONFIG_REFCOUNT_FULL=y
From: Song Liu @ 2018-05-08 21:09 UTC (permalink / raw)
  To: Martin Lau; +Cc: Networking, Alexei Starovoitov, Daniel Borkmann, Kernel Team
In-Reply-To: <20180504214955.1058805-2-kafai@fb.com>



> On May 4, 2018, at 2:49 PM, Martin KaFai Lau <kafai@fb.com> wrote:
> 
> If CONFIG_REFCOUNT_FULL=y, refcount_inc() WARN when refcount is 0.
> When creating a new btf, the initial btf->refcnt is 0 and
> triggered the following:
> 
> [   34.855452] refcount_t: increment on 0; use-after-free.
> [   34.856252] WARNING: CPU: 6 PID: 1857 at lib/refcount.c:153 refcount_inc+0x26/0x30
> ....
> [   34.868809] Call Trace:
> [   34.869168]  btf_new_fd+0x1af6/0x24d0
> [   34.869645]  ? btf_type_seq_show+0x200/0x200
> [   34.870212]  ? lock_acquire+0x3b0/0x3b0
> [   34.870726]  ? security_capable+0x54/0x90
> [   34.871247]  __x64_sys_bpf+0x1b2/0x310
> [   34.871761]  ? __ia32_sys_bpf+0x310/0x310
> [   34.872285]  ? bad_area_access_error+0x310/0x310
> [   34.872894]  do_syscall_64+0x95/0x3f0
> 
> This patch uses refcount_set() instead.
> 
> Reported-by: Yonghong Song <yhs@fb.com>
> Tested-by: Yonghong Song <yhs@fb.com>
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> ---
> kernel/bpf/btf.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 22e1046a1a86..fa0dce0452e7 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -1977,7 +1977,7 @@ static struct btf *btf_parse(void __user *btf_data, u32 btf_data_size,
> 
> 	if (!err) {
> 		btf_verifier_env_free(env);
> -		btf_get(btf);
> +		refcount_set(&btf->refcnt, 1);
> 		return btf;
> 	}
> 
> -- 
> 2.9.5
> 

Acked-by: Song Liu <songliubraving@fb.com>

^ permalink raw reply

* Re: [PATCH bpf-next 3/6] bpf: btf: Add struct bpf_btf_info
From: Song Liu @ 2018-05-08 21:09 UTC (permalink / raw)
  To: Martin Lau
  Cc: netdev@vger.kernel.org, Alexei Starovoitov, Daniel Borkmann,
	Kernel Team
In-Reply-To: <20180504214955.1058805-4-kafai@fb.com>



> On May 4, 2018, at 2:49 PM, Martin KaFai Lau <kafai@fb.com> wrote:
> 
> During BPF_OBJ_GET_INFO_BY_FD on a btf_fd, the current bpf_attr's
> info.info is directly filled with the BTF binary data.  It is
> not extensible.  In this case, we want to add BTF ID.
> 
> This patch adds "struct bpf_btf_info" which has the BTF ID as
> one of its member.  The BTF binary data itself is exposed through
> the "btf" and "btf_size" members.
> 
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> Acked-by: Alexei Starovoitov <ast@fb.com>
> ---
> include/uapi/linux/bpf.h |  6 ++++++
> kernel/bpf/btf.c         | 26 +++++++++++++++++++++-----
> kernel/bpf/syscall.c     | 17 ++++++++++++++++-
> 3 files changed, 43 insertions(+), 6 deletions(-)
> 
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 6106f23a9a8a..d615c777b573 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -2137,6 +2137,12 @@ struct bpf_map_info {
> 	__u32 btf_value_id;
> } __attribute__((aligned(8)));
> 
> +struct bpf_btf_info {
> +	__aligned_u64 btf;
> +	__u32 btf_size;
> +	__u32 id;
> +} __attribute__((aligned(8)));
> +
> /* User bpf_sock_addr struct to access socket fields and sockaddr struct passed
>  * by user and intended to be used by socket (e.g. to bind to, depends on
>  * attach attach type).
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 40950b6bf395..ded10ab47b8a 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -2114,12 +2114,28 @@ int btf_get_info_by_fd(const struct btf *btf,
> 		       const union bpf_attr *attr,
> 		       union bpf_attr __user *uattr)
> {
> -	void __user *udata = u64_to_user_ptr(attr->info.info);
> -	u32 copy_len = min_t(u32, btf->data_size,
> -			     attr->info.info_len);
> +	struct bpf_btf_info __user *uinfo;
> +	struct bpf_btf_info info = {};
> +	u32 info_copy, btf_copy;
> +	void __user *ubtf;
> +	u32 uinfo_len;
> 
> -	if (copy_to_user(udata, btf->data, copy_len) ||
> -	    put_user(btf->data_size, &uattr->info.info_len))
> +	uinfo = u64_to_user_ptr(attr->info.info);
> +	uinfo_len = attr->info.info_len;
> +
> +	info_copy = min_t(u32, uinfo_len, sizeof(info));
> +	if (copy_from_user(&info, uinfo, info_copy))
> +		return -EFAULT;
> +
> +	info.id = btf->id;
> +	ubtf = u64_to_user_ptr(info.btf);
> +	btf_copy = min_t(u32, btf->data_size, info.btf_size);
> +	if (copy_to_user(ubtf, btf->data, btf_copy))
> +		return -EFAULT;
> +	info.btf_size = btf->data_size;
> +
> +	if (copy_to_user(uinfo, &info, info_copy) ||
> +	    put_user(info_copy, &uattr->info.info_len))
> 		return -EFAULT;
> 
> 	return 0;
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 8b0a45d65454..d2895e3e5cbf 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -2019,6 +2019,21 @@ static int bpf_map_get_info_by_fd(struct bpf_map *map,
> 	return 0;
> }
> 
> +static int bpf_btf_get_info_by_fd(struct btf *btf,
> +				  const union bpf_attr *attr,
> +				  union bpf_attr __user *uattr)
> +{
> +	struct bpf_btf_info __user *uinfo = u64_to_user_ptr(attr->info.info);
> +	u32 info_len = attr->info.info_len;
> +	int err;
> +
> +	err = check_uarg_tail_zero(uinfo, sizeof(*uinfo), info_len);
> +	if (err)
> +		return err;
> +
> +	return btf_get_info_by_fd(btf, attr, uattr);
> +}
> +
> #define BPF_OBJ_GET_INFO_BY_FD_LAST_FIELD info.info
> 
> static int bpf_obj_get_info_by_fd(const union bpf_attr *attr,
> @@ -2042,7 +2057,7 @@ static int bpf_obj_get_info_by_fd(const union bpf_attr *attr,
> 		err = bpf_map_get_info_by_fd(f.file->private_data, attr,
> 					     uattr);
> 	else if (f.file->f_op == &btf_fops)
> -		err = btf_get_info_by_fd(f.file->private_data, attr, uattr);
> +		err = bpf_btf_get_info_by_fd(f.file->private_data, attr, uattr);
> 	else
> 		err = -EINVAL;
> 
> -- 
> 2.9.5
> 

Acked-by: Song Liu <songliubraving@fb.com>

^ permalink raw reply

* Re: [PATCH bpf-next 2/6] bpf: btf: Introduce BTF ID
From: Song Liu @ 2018-05-08 21:09 UTC (permalink / raw)
  To: Martin Lau
  Cc: netdev@vger.kernel.org, Alexei Starovoitov, Daniel Borkmann,
	Kernel Team
In-Reply-To: <20180504214955.1058805-3-kafai@fb.com>



> On May 4, 2018, at 2:49 PM, Martin KaFai Lau <kafai@fb.com> wrote:
> 
> This patch gives an ID to each loaded BTF.  The ID is allocated by
> the idr like the existing prog-id and map-id.
> 
> The bpf_put(map->btf) is moved to __bpf_map_put() so that the
> userspace can stop seeing the BTF ID ASAP when the last BTF
> refcnt is gone.
> 
> It also makes BTF accessible from userspace through the
> 1. new BPF_BTF_GET_FD_BY_ID command.  It is limited to CAP_SYS_ADMIN
>   which is inline with the BPF_BTF_LOAD cmd and the existing
>   BPF_[MAP|PROG]_GET_FD_BY_ID cmd.
> 2. new btf_id (and btf_key_id + btf_value_id) in "struct bpf_map_info"
> 
> Once the BTF ID handler is accessible from userspace, freeing a BTF
> object has to go through a rcu period.  The BPF_BTF_GET_FD_BY_ID cmd
> can then be done under a rcu_read_lock() instead of taking
> spin_lock.
> [Note: A similar rcu usage can be done to the existing
>       bpf_prog_get_fd_by_id() in a follow up patch]
> 
> When processing the BPF_BTF_GET_FD_BY_ID cmd,
> refcount_inc_not_zero() is needed because the BTF object
> could be already in the rcu dead row .  btf_get() is
> removed since its usage is currently limited to btf.c
> alone.  refcount_inc() is used directly instead.
> 
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> Acked-by: Alexei Starovoitov <ast@fb.com>
> ---
> include/linux/btf.h      |   2 +
> include/uapi/linux/bpf.h |   5 +++
> kernel/bpf/btf.c         | 108 ++++++++++++++++++++++++++++++++++++++++++-----
> kernel/bpf/syscall.c     |  24 ++++++++++-
> 4 files changed, 128 insertions(+), 11 deletions(-)
> 
> diff --git a/include/linux/btf.h b/include/linux/btf.h
> index a966dc6d61ee..e076c4697049 100644
> --- a/include/linux/btf.h
> +++ b/include/linux/btf.h
> @@ -44,5 +44,7 @@ const struct btf_type *btf_type_id_size(const struct btf *btf,
> 					u32 *ret_size);
> void btf_type_seq_show(const struct btf *btf, u32 type_id, void *obj,
> 		       struct seq_file *m);
> +int btf_get_fd_by_id(u32 id);
> +u32 btf_id(const struct btf *btf);
> 
> #endif
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 93d5a4eeec2a..6106f23a9a8a 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -96,6 +96,7 @@ enum bpf_cmd {
> 	BPF_PROG_QUERY,
> 	BPF_RAW_TRACEPOINT_OPEN,
> 	BPF_BTF_LOAD,
> +	BPF_BTF_GET_FD_BY_ID,
> };
> 
> enum bpf_map_type {
> @@ -344,6 +345,7 @@ union bpf_attr {
> 			__u32		start_id;
> 			__u32		prog_id;
> 			__u32		map_id;
> +			__u32		btf_id;
> 		};
> 		__u32		next_id;
> 		__u32		open_flags;
> @@ -2130,6 +2132,9 @@ struct bpf_map_info {
> 	__u32 ifindex;
> 	__u64 netns_dev;
> 	__u64 netns_ino;
> +	__u32 btf_id;
> +	__u32 btf_key_id;
> +	__u32 btf_value_id;
> } __attribute__((aligned(8)));
> 
> /* User bpf_sock_addr struct to access socket fields and sockaddr struct passed
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index fa0dce0452e7..40950b6bf395 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -11,6 +11,7 @@
> #include <linux/file.h>
> #include <linux/uaccess.h>
> #include <linux/kernel.h>
> +#include <linux/idr.h>
> #include <linux/bpf_verifier.h>
> #include <linux/btf.h>
> 
> @@ -179,6 +180,9 @@
> 	     i < btf_type_vlen(struct_type);				\
> 	     i++, member++)
> 
> +static DEFINE_IDR(btf_idr);
> +static DEFINE_SPINLOCK(btf_idr_lock);
> +
> struct btf {
> 	union {
> 		struct btf_header *hdr;
> @@ -193,6 +197,8 @@ struct btf {
> 	u32 types_size;
> 	u32 data_size;
> 	refcount_t refcnt;
> +	u32 id;
> +	struct rcu_head rcu;
> };
> 
> enum verifier_phase {
> @@ -598,6 +604,42 @@ static int btf_add_type(struct btf_verifier_env *env, struct btf_type *t)
> 	return 0;
> }
> 
> +static int btf_alloc_id(struct btf *btf)
> +{
> +	int id;
> +
> +	idr_preload(GFP_KERNEL);
> +	spin_lock_bh(&btf_idr_lock);
> +	id = idr_alloc_cyclic(&btf_idr, btf, 1, INT_MAX, GFP_ATOMIC);
> +	if (id > 0)
> +		btf->id = id;
> +	spin_unlock_bh(&btf_idr_lock);
> +	idr_preload_end();
> +
> +	if (WARN_ON_ONCE(!id))
> +		return -ENOSPC;
> +
> +	return id > 0 ? 0 : id;
> +}
> +
> +static void btf_free_id(struct btf *btf)
> +{
> +	unsigned long flags;
> +
> +	/*
> +	 * In map-in-map, calling map_delete_elem() on outer
> +	 * map will call bpf_map_put on the inner map.
> +	 * It will then eventually call btf_free_id()
> +	 * on the inner map.  Some of the map_delete_elem()
> +	 * implementation may have irq disabled, so
> +	 * we need to use the _irqsave() version instead
> +	 * of the _bh() version.
> +	 */
> +	spin_lock_irqsave(&btf_idr_lock, flags);
> +	idr_remove(&btf_idr, btf->id);
> +	spin_unlock_irqrestore(&btf_idr_lock, flags);
> +}
> +
> static void btf_free(struct btf *btf)
> {
> 	kvfree(btf->types);
> @@ -607,15 +649,19 @@ static void btf_free(struct btf *btf)
> 	kfree(btf);
> }
> 
> -static void btf_get(struct btf *btf)
> +static void btf_free_rcu(struct rcu_head *rcu)
> {
> -	refcount_inc(&btf->refcnt);
> +	struct btf *btf = container_of(rcu, struct btf, rcu);
> +
> +	btf_free(btf);
> }
> 
> void btf_put(struct btf *btf)
> {
> -	if (btf && refcount_dec_and_test(&btf->refcnt))
> -		btf_free(btf);
> +	if (btf && refcount_dec_and_test(&btf->refcnt)) {
> +		btf_free_id(btf);
> +		call_rcu(&btf->rcu, btf_free_rcu);
> +	}
> }
> 
> static int env_resolve_init(struct btf_verifier_env *env)
> @@ -2006,10 +2052,15 @@ const struct file_operations btf_fops = {
> 	.release	= btf_release,
> };
> 
> +static int __btf_new_fd(struct btf *btf)
> +{
> +	return anon_inode_getfd("btf", &btf_fops, btf, O_RDONLY | O_CLOEXEC);
> +}
> +
> int btf_new_fd(const union bpf_attr *attr)
> {
> 	struct btf *btf;
> -	int fd;
> +	int ret;
> 
> 	btf = btf_parse(u64_to_user_ptr(attr->btf),
> 			attr->btf_size, attr->btf_log_level,
> @@ -2018,12 +2069,23 @@ int btf_new_fd(const union bpf_attr *attr)
> 	if (IS_ERR(btf))
> 		return PTR_ERR(btf);
> 
> -	fd = anon_inode_getfd("btf", &btf_fops, btf,
> -			      O_RDONLY | O_CLOEXEC);
> -	if (fd < 0)
> +	ret = btf_alloc_id(btf);
> +	if (ret) {
> +		btf_free(btf);
> +		return ret;
> +	}
> +
> +	/*
> +	 * The BTF ID is published to the userspace.
> +	 * All BTF free must go through call_rcu() from
> +	 * now on (i.e. free by calling btf_put()).
> +	 */
> +
> +	ret = __btf_new_fd(btf);
> +	if (ret < 0)
> 		btf_put(btf);
> 
> -	return fd;
> +	return ret;
> }
> 
> struct btf *btf_get_by_fd(int fd)
> @@ -2042,7 +2104,7 @@ struct btf *btf_get_by_fd(int fd)
> 	}
> 
> 	btf = f.file->private_data;
> -	btf_get(btf);
> +	refcount_inc(&btf->refcnt);
> 	fdput(f);
> 
> 	return btf;
> @@ -2062,3 +2124,29 @@ int btf_get_info_by_fd(const struct btf *btf,
> 
> 	return 0;
> }
> +
> +int btf_get_fd_by_id(u32 id)
> +{
> +	struct btf *btf;
> +	int fd;
> +
> +	rcu_read_lock();
> +	btf = idr_find(&btf_idr, id);
> +	if (!btf || !refcount_inc_not_zero(&btf->refcnt))
> +		btf = ERR_PTR(-ENOENT);
> +	rcu_read_unlock();
> +
> +	if (IS_ERR(btf))
> +		return PTR_ERR(btf);
> +
> +	fd = __btf_new_fd(btf);
> +	if (fd < 0)
> +		btf_put(btf);
> +
> +	return fd;
> +}
> +
> +u32 btf_id(const struct btf *btf)
> +{
> +	return btf->id;
> +}
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 263e13ede029..8b0a45d65454 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -252,7 +252,6 @@ static void bpf_map_free_deferred(struct work_struct *work)
> 
> 	bpf_map_uncharge_memlock(map);
> 	security_bpf_map_free(map);
> -	btf_put(map->btf);
> 	/* implementation dependent freeing */
> 	map->ops->map_free(map);
> }
> @@ -273,6 +272,7 @@ static void __bpf_map_put(struct bpf_map *map, bool do_idr_lock)
> 	if (atomic_dec_and_test(&map->refcnt)) {
> 		/* bpf_map_free_id() must be called first */
> 		bpf_map_free_id(map, do_idr_lock);
> +		btf_put(map->btf);
> 		INIT_WORK(&map->work, bpf_map_free_deferred);
> 		schedule_work(&map->work);
> 	}
> @@ -2000,6 +2000,12 @@ static int bpf_map_get_info_by_fd(struct bpf_map *map,
> 	info.map_flags = map->map_flags;
> 	memcpy(info.name, map->name, sizeof(map->name));
> 
> +	if (map->btf) {
> +		info.btf_id = btf_id(map->btf);
> +		info.btf_key_id = map->btf_key_id;
> +		info.btf_value_id = map->btf_value_id;
> +	}
> +
> 	if (bpf_map_is_dev_bound(map)) {
> 		err = bpf_map_offload_info_fill(&info, map);
> 		if (err)
> @@ -2057,6 +2063,19 @@ static int bpf_btf_load(const union bpf_attr *attr)
> 	return btf_new_fd(attr);
> }
> 
> +#define BPF_BTF_GET_FD_BY_ID_LAST_FIELD btf_id
> +
> +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))
> +		return -EPERM;
> +
> +	return btf_get_fd_by_id(attr->btf_id);
> +}
> +
> SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, size)
> {
> 	union bpf_attr attr = {};
> @@ -2140,6 +2159,9 @@ SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, siz
> 	case BPF_BTF_LOAD:
> 		err = bpf_btf_load(&attr);
> 		break;
> +	case BPF_BTF_GET_FD_BY_ID:
> +		err = bpf_btf_get_fd_by_id(&attr);
> +		break;
> 	default:
> 		err = -EINVAL;
> 		break;
> -- 
> 2.9.5
> 


Acked-by: Song Liu <songliubraving@fb.com>

^ permalink raw reply

* Re: [PATCH net-next] dt-bindings: dsa: Remove unnecessary #address/#size-cells
From: Florian Fainelli @ 2018-05-08 20:58 UTC (permalink / raw)
  To: Fabio Estevam, davem; +Cc: andrew, robh+dt, netdev, devicetree, Fabio Estevam
In-Reply-To: <1525695471-19984-1-git-send-email-festevam@gmail.com>

On 05/07/2018 05:17 AM, Fabio Estevam wrote:
> From: Fabio Estevam <fabio.estevam@nxp.com>
> 
> If the example binding is used on a real dts file, the following DTC
> warning is seen with W=1:
>     
> arch/arm/boot/dts/imx6q-b450v3.dtb: Warning (avoid_unnecessary_addr_size): /mdio-gpio/switch@0: unnecessary #address-cells/#size-cells without "ranges" or child "reg" property
> 
> Remove unnecessary #address-cells/#size-cells to improve the binding
> document examples.

In most cases this is unnecessary because the parent node is an MDIO,
I2C or SPI controller, and those typically have #address-cells = <1> and
#size-cells = <0> because of their specific binding, but this is not
necessarily true if using e.g: a MMIO mapped Ethernet switch.

With the particular example though, this appears fine:

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

^ permalink raw reply

* Re: pull-request: ieee802154 2018-05-08
From: Stefan Schmidt @ 2018-05-08 19:55 UTC (permalink / raw)
  To: David Miller, s.schmidt; +Cc: linux-wpan, alex.aring, netdev
In-Reply-To: <20180508.101808.776198033126419650.davem@davemloft.net>

Hello.


On 05/08/2018 04:18 PM, David Miller wrote:
> From: Stefan Schmidt <s.schmidt@samsung.com>
> Date: Tue,  8 May 2018 10:29:27 +0200
>
>> An update from ieee802154 for your *net* tree.
>>
>> Two fixes for the mcr20a driver, which was being added in the 4.17 merge window,
>> by Gustavo and myself.
>> The atusb driver got a change to GFP_KERNEL where no GFP_ATOMIC is needed by
>> Jia-Ju.
>>
>> The last and most important fix is from Alex to get IPv6 reassembly working
>> again for the ieee802154 6lowpan adaptation. This got broken in 4.16 so please
>> queue this one also up for the 4.16 stable tree.
> Pulled, thanks.

Thanks.
>
> Please submit the -stable fix directly, you can feel free to CC: me.

Will do when the patch hits Linus git tree.

I have a quick question on the process here. From the netdev-faq document
I was under the impression all stable patches under net/ and drivers/net
should be brought up to you and would be handled by you.

Does this apply to the core part of net (I fully understand that ieee802154
is rather a niche) or is there some other reason for this exception?

Both processes (the normal stable one as well as the slightly different one
for net/) would be fine to go with for me. Just need to know which one I
should use for future stable patches. :-)

regards
Stefan Schmidt

^ permalink raw reply

* Re: KASAN: use-after-free Read in sctp_do_sm
From: Marcelo Ricardo Leitner @ 2018-05-08 18:57 UTC (permalink / raw)
  To: Xin Long
  Cc: syzbot, davem, LKML, linux-sctp, network dev, Neil Horman,
	syzkaller-bugs, Vlad Yasevich
In-Reply-To: <CADvbK_d8O7b9OCfCKGhaaQuBafi6rgkKEQh9Bk7pvZKX3KDKCg@mail.gmail.com>

On Wed, May 09, 2018 at 01:41:03AM +0800, Xin Long wrote:
...
> >  sctp_chunk_destroy net/sctp/sm_make_chunk.c:1481 [inline]
> >  sctp_chunk_put+0x321/0x440 net/sctp/sm_make_chunk.c:1504
> >  sctp_ulpevent_make_rcvmsg+0x955/0xd40 net/sctp/ulpevent.c:718
> There's no reason to put the chunk in sctp_ulpevent_make_rcvmsg's
> fail_mark err path before holding this chunk later there.
>
> We should just remove it.

Oups. Agreed.

  Marcelo

^ permalink raw reply

* Re: [PATCH v6 13/13] Documentation: clarify firmware_class provenance and why we can't rename the module
From: Andres Rodriguez @ 2018-05-08 18:53 UTC (permalink / raw)
  To: Luis R. Rodriguez, gregkh
  Cc: andresx7, akpm, keescook, josh, maco, andy.gross, david.brown,
	bjorn.andersson, teg, wagi, hdegoede, zohar, kubakici, shuah,
	mfuzzey, dhowells, pali.rohar, tiwai, kvalo, arend.vanspriel,
	zajec5, nbroeking, markivx, broonie, dmitry.torokhov, dwmw2,
	torvalds, Abhay_Salunke, jewalt, oneukum, cantabile.desu, ast,
	hare, jejb, martin.petersen, khc, davem
In-Reply-To: <20180508181247.19431-14-mcgrof@kernel.org>



On 2018-05-08 02:12 PM, Luis R. Rodriguez wrote:
> Clarify the provenance of the firmware loader firmware_class module name
> and why we cannot rename the module in the future.
> 
> Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
> ---
>   .../driver-api/firmware/fallback-mechanisms.rst          | 9 ++++++---
>   1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/driver-api/firmware/fallback-mechanisms.rst b/Documentation/driver-api/firmware/fallback-mechanisms.rst
> index a39323ef7d29..a8047be4a96e 100644
> --- a/Documentation/driver-api/firmware/fallback-mechanisms.rst
> +++ b/Documentation/driver-api/firmware/fallback-mechanisms.rst
> @@ -72,9 +72,12 @@ the firmware requested, and establishes it in the device hierarchy by
>   associating the device used to make the request as the device's parent.
>   The sysfs directory's file attributes are defined and controlled through
>   the new device's class (firmware_class) and group (fw_dev_attr_groups).
> -This is actually where the original firmware_class.c file name comes from,
> -as originally the only firmware loading mechanism available was the
> -mechanism we now use as a fallback mechanism.
> +This is actually where the original firmware_class module name came from,
> +given that originally the only firmware loading mechanism available was the
> +mechanism we now use as a fallback mechanism, which which registers a

Just a tiny repeated word here, "which which".

-Andres


> +struct class firmware_class. Because the attributes exposed are part of the
> +module name, the module name firmware_class cannot be renamed in the future, to
> +ensure backward compatibilty with old userspace.
>   
>   To load firmware using the sysfs interface we expose a loading indicator,
>   and a file upload firmware into:
> 

^ permalink raw reply

* Re: [PATCH] selinux: add AF_UNSPEC and INADDR_ANY checks to selinux_socket_bind()
From: Stephen Smalley @ 2018-05-08 18:40 UTC (permalink / raw)
  To: Paul Moore, Alexey Kodanev, Richard Haines
  Cc: selinux, Eric Paris, linux-security-module, netdev
In-Reply-To: <CAHC9VhTACGPt2dSkUN9Efxs-HQVSAsxoiwPxHcujd++O-mMafg@mail.gmail.com>

On 05/08/2018 01:05 PM, Paul Moore wrote:
> On Tue, May 8, 2018 at 10:05 AM, Alexey Kodanev
> <alexey.kodanev@oracle.com> wrote:
>> Commit d452930fd3b9 ("selinux: Add SCTP support") breaks compatibility
>> with the old programs that can pass sockaddr_in with AF_UNSPEC and
>> INADDR_ANY to bind(). As a result, bind() returns EAFNOSUPPORT error.
>> It was found with LTP/asapi_01 test.
>>
>> Similar to commit 29c486df6a20 ("net: ipv4: relax AF_INET check in
>> bind()"), which relaxed AF_INET check for compatibility, add AF_UNSPEC
>> case to AF_INET and make sure that the address is INADDR_ANY.
>>
>> Also, in the end of selinux_socket_bind(), instead of adding AF_UNSPEC
>> to 'address->sa_family == AF_INET', verify AF_INET6 first.
>>
>> Fixes: d452930fd3b9 ("selinux: Add SCTP support")
>> Signed-off-by: Alexey Kodanev <alexey.kodanev@oracle.com>
>> ---
>>  security/selinux/hooks.c | 12 +++++++++---
>>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> Thanks for finding and reporting this regression.
> 
> I think I would prefer to avoid having to duplicate the
> AF_UNSPEC/INADDR_ANY checking logic in the SELinux hook, even though
> it is a small bit of code and unlikely to change.  I'm wondering if it
> would be better to check both the socket and sockaddr address family
> in the main if conditional inside selinux_socket_bind(), what do you
> think?  Another option would be to go back to just checking the
> soackaddr address family; we moved away from that for a reason which
> escapes at the moment (code cleanliness?), but perhaps that was a
> mistake.

We've always used the sk->sk_family there; it was only the recent code from Richard that started
using the socket address family.

> 
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 4cafe6a19167..a3789b167667 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -4577,6 +4577,7 @@ static int selinux_socket_bind(struct socket *sock, struc>
> {
>        struct sock *sk = sock->sk;
>        u16 family;
> +       u16 family_sa;
>        int err;
> 
>        err = sock_has_perm(sk, SOCKET__BIND);
> @@ -4585,7 +4586,9 @@ static int selinux_socket_bind(struct socket *sock, struc>
> 
>        /* If PF_INET or PF_INET6, check name_bind permission for the port. */
>        family = sk->sk_family;
> -       if (family == PF_INET || family == PF_INET6) {
> +       family_sa = address->sa_family;
> +       if ((family == PF_INET || family == PF_INET6) &&
> +           (family_sa == PF_INET || family_sa == PF_INET6)) {

Wouldn't this allow bypassing the name_bind permission check by passing in AF_UNSPEC?

>                char *addrp;
>                struct sk_security_struct *sksec = sk->sk_security;
>                struct common_audit_data ad;
> @@ -4601,7 +4604,7 @@ static int selinux_socket_bind(struct socket *sock, struc>
>                 * need to check address->sa_family as it is possible to have
>                 * sk->sk_family = PF_INET6 with addr->sa_family = AF_INET.
>                 */
> -               switch (address->sa_family) {
> +               switch (family_sa) {
>                case AF_INET:
>                        if (addrlen < sizeof(struct sockaddr_in))
>                                return -EINVAL;
> 
>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>> index 4cafe6a..649a3be 100644
>> --- a/security/selinux/hooks.c
>> +++ b/security/selinux/hooks.c
>> @@ -4602,10 +4602,16 @@ static int selinux_socket_bind(struct socket *sock, struct sockaddr *address, in
>>                  * sk->sk_family = PF_INET6 with addr->sa_family = AF_INET.
>>                  */
>>                 switch (address->sa_family) {
>> +               case AF_UNSPEC:
>>                 case AF_INET:
>>                         if (addrlen < sizeof(struct sockaddr_in))
>>                                 return -EINVAL;
>>                         addr4 = (struct sockaddr_in *)address;
>> +
>> +                       if (address->sa_family == AF_UNSPEC &&
>> +                           addr4->sin_addr.s_addr != htonl(INADDR_ANY))
>> +                               return -EAFNOSUPPORT;
>> +
>>                         snum = ntohs(addr4->sin_port);
>>                         addrp = (char *)&addr4->sin_addr.s_addr;
>>                         break;
>> @@ -4681,10 +4687,10 @@ static int selinux_socket_bind(struct socket *sock, struct sockaddr *address, in
>>                 ad.u.net->sport = htons(snum);
>>                 ad.u.net->family = family;
>>
>> -               if (address->sa_family == AF_INET)
>> -                       ad.u.net->v4info.saddr = addr4->sin_addr.s_addr;
>> -               else
>> +               if (address->sa_family == AF_INET6)
>>                         ad.u.net->v6info.saddr = addr6->sin6_addr;
>> +               else
>> +                       ad.u.net->v4info.saddr = addr4->sin_addr.s_addr;
>>
>>                 err = avc_has_perm(&selinux_state,
>>                                    sksec->sid, sid,
>> --
>> 1.8.3.1
>>
> 

^ permalink raw reply

* Re: Failed to clone net-next.git
From: Song Liu @ 2018-05-08 18:37 UTC (permalink / raw)
  To: Konstantin Ryabitsev; +Cc: Networking, Alexei Starovoitov
In-Reply-To: <f5d5d5c3-3b15-ddbe-45c6-9f360f1042bf@linuxfoundation.org>



> On May 8, 2018, at 11:06 AM, Konstantin Ryabitsev <konstantin@linuxfoundation.org> wrote:
> 
> On 05/08/18 13:46, Song Liu wrote:
>> We are seeing the following error on multiple different systems while 
>> cloning net-next tree. 
>> 
>>  $ git clone https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git
>>  Cloning into 'net-next'...
>>  remote: Counting objects: 6017287, done.
>>  remote: Compressing objects: 100% (2458/2458), done.
>>  fatal: The remote end hung up unexpectedly 1.00 GiB | 8.13 MiB/s
>>  fatal: early EOF
>>  fatal: index-pack failed
>> 
>> It looks like the size of the data being fetched reached server side
>> limit of 1.00 GiB. So we probably need change server side configuration.
>> Could someone please look into it?
> 
> It was probably due to a timeout value. Can you try it now, I've bumped
> it to a much larger number.
> 
> Regards,
> -- 
> Konstantin Ryabitsev
> Director, IT Infrastructure Security
> The Linux Foundation

Thanks Konstantin. It works for me now. 

By the way, I do need to increase http.postBuffer on the client side:

git config --global http.postBuffer "something > 1GiB"

Song

^ permalink raw reply

* [PATCH net 2/2] qede: Fix gfp flags sent to rdma event node allocation
From: Michal Kalderon @ 2018-05-08 18:29 UTC (permalink / raw)
  To: michal.kalderon, davem
  Cc: netdev, linux-rdma, chad.dupuis, Michal Kalderon,
	Sudarsana Kalluru
In-Reply-To: <20180508182919.23590-1-Michal.Kalderon@cavium.com>

A previous commit 4609adc27175 ("qede: Fix qedr link update")
added a flow that could allocate rdma event objects from an
interrupt path (link notification). Therefore the kzalloc call
should be done with GFP_ATOMIC.

fixes: 4609adc27175 ("qede: Fix qedr link update")
Signed-off-by: Michal Kalderon <Michal.Kalderon@cavium.com>
Signed-off-by: Sudarsana Kalluru <Sudarsana.Kalluru@cavium.com>
---
 drivers/net/ethernet/qlogic/qede/qede_rdma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/qlogic/qede/qede_rdma.c b/drivers/net/ethernet/qlogic/qede/qede_rdma.c
index 50b142f..1900bf7 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_rdma.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_rdma.c
@@ -238,7 +238,7 @@ qede_rdma_get_free_event_node(struct qede_dev *edev)
 	}
 
 	if (!found) {
-		event_node = kzalloc(sizeof(*event_node), GFP_KERNEL);
+		event_node = kzalloc(sizeof(*event_node), GFP_ATOMIC);
 		if (!event_node) {
 			DP_NOTICE(edev,
 				  "qedr: Could not allocate memory for rdma work\n");
-- 
2.9.5

^ permalink raw reply related

* [PATCH net 1/2] qed: Fix l2 initializations over iWARP personality
From: Michal Kalderon @ 2018-05-08 18:29 UTC (permalink / raw)
  To: michal.kalderon, davem
  Cc: netdev, linux-rdma, chad.dupuis, Michal Kalderon,
	Sudarsana Kalluru
In-Reply-To: <20180508182919.23590-1-Michal.Kalderon@cavium.com>

If qede driver was loaded on a device configured for iWARP
the l2 mutex wouldn't be allocated, and some l2 related
resources wouldn't be freed.

fixes: c851a9dc4359 ("qed: Introduce iWARP personality")
Signed-off-by: Michal Kalderon <Michal.Kalderon@cavium.com>
Signed-off-by: Sudarsana Kalluru <Sudarsana.Kalluru@cavium.com>
---
 drivers/net/ethernet/qlogic/qed/qed_l2.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qed/qed_l2.c b/drivers/net/ethernet/qlogic/qed/qed_l2.c
index e874504..8667799 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_l2.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_l2.c
@@ -115,8 +115,7 @@ int qed_l2_alloc(struct qed_hwfn *p_hwfn)
 
 void qed_l2_setup(struct qed_hwfn *p_hwfn)
 {
-	if (p_hwfn->hw_info.personality != QED_PCI_ETH &&
-	    p_hwfn->hw_info.personality != QED_PCI_ETH_ROCE)
+	if (!QED_IS_L2_PERSONALITY(p_hwfn))
 		return;
 
 	mutex_init(&p_hwfn->p_l2_info->lock);
@@ -126,8 +125,7 @@ void qed_l2_free(struct qed_hwfn *p_hwfn)
 {
 	u32 i;
 
-	if (p_hwfn->hw_info.personality != QED_PCI_ETH &&
-	    p_hwfn->hw_info.personality != QED_PCI_ETH_ROCE)
+	if (!QED_IS_L2_PERSONALITY(p_hwfn))
 		return;
 
 	if (!p_hwfn->p_l2_info)
-- 
2.9.5

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox