* Re: [PATCH bpf-next 02/10] libbpf: implement BPF CO-RE offset relocation algorithm
From: Yonghong Song @ 2019-07-27 21:29 UTC (permalink / raw)
To: Andrii Nakryiko, Alexei Starovoitov
Cc: Alexei Starovoitov, Andrii Nakryiko, bpf, Networking,
Daniel Borkmann, Kernel Team
In-Reply-To: <CAEf4Bzbt4+mT8GfQG4xMj4tCnWd2ZqJiY3r8cwOankFFQo8wWA@mail.gmail.com>
On 7/27/19 11:24 AM, Andrii Nakryiko wrote:
> On Sat, Jul 27, 2019 at 10:00 AM Alexei Starovoitov <ast@fb.com> wrote:
>>
>> On 7/26/19 11:25 PM, Andrii Nakryiko wrote:
>>>>> + } else if (class == BPF_ST && BPF_MODE(insn->code) == BPF_MEM) {
>>>>> + if (insn->imm != orig_off)
>>>>> + return -EINVAL;
>>>>> + insn->imm = new_off;
>>>>> + pr_debug("prog '%s': patched insn #%d (ST | MEM) imm %d -> %d\n",
>>>>> + bpf_program__title(prog, false),
>>>>> + insn_idx, orig_off, new_off);
>>>> I'm pretty sure llvm was not capable of emitting BPF_ST insn.
>>>> When did that change?
>>> I just looked at possible instructions that could have 32-bit
>>> immediate value. This is `*(rX) = offsetof(struct s, field)`, which I
>>> though is conceivable. Do you think I should drop it?
>>
>> Just trying to point out that since it's not emitted by llvm
>> this code is likely untested ?
>> Or you've created a bpf asm test for this?
>
>
> Yeah, it's untested right now. Let me try to come up with LLVM
> assembly + relocation (not yet sure how/whether builtin works with
> inline assembly), if that works out, I'll leave this, if not, I'll
> drop BPF_ST|BPF_MEM part.
FYI. The llvm does not have assembly code format for BPF_ST instructions
as it does not generate code for it. So inline asm through llvm won't
work. llvm disasseembler won't be able to decode BPF_ST either.
>>
>>
^ permalink raw reply
* Re: [PATCH net] net: phylink: Fix flow control for fixed-link
From: David Miller @ 2019-07-27 21:28 UTC (permalink / raw)
To: opensource; +Cc: linux, andrew, f.fainelli, hkallweit1, netdev
In-Reply-To: <20190727094011.14024-1-opensource@vdorst.com>
From: René van Dorst <opensource@vdorst.com>
Date: Sat, 27 Jul 2019 11:40:11 +0200
> In phylink_parse_fixedlink() the pl->link_config.advertising bits are AND
> with pl->supported, pl->supported is zeroed and only the speed/duplex
> modes and MII bits are set.
> So pl->link_config.advertising always loses the flow control/pause bits.
>
> By setting Pause and Asym_Pause bits in pl->supported, the flow control
> work again when devicetree "pause" is set in fixes-link node and the MAC
> advertise that is supports pause.
>
> Results with this patch.
>
> Legend:
> - DT = 'Pause' is set in the fixed-link in devicetree.
> - validate() = ‘Yes’ means phylink_set(mask, Pause) is set in the
> validate().
> - flow = results reported my link is Up line.
>
> +-----+------------+-------+
> | DT | validate() | flow |
> +-----+------------+-------+
> | Yes | Yes | rx/tx |
> | No | Yes | off |
> | Yes | No | off |
> +-----+------------+-------+
>
> Fixes: 9525ae83959b ("phylink: add phylink infrastructure")
> Signed-off-by: René van Dorst <opensource@vdorst.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH] gigaset: stop maintaining seperately
From: David Miller @ 2019-07-27 21:27 UTC (permalink / raw)
To: pebolle; +Cc: tilman, hjlipp, arnd, kkeil, netdev, linux-kernel
In-Reply-To: <20190726220541.28783-1-pebolle@tiscali.nl>
From: Paul Bolle <pebolle@tiscali.nl>
Date: Sat, 27 Jul 2019 00:05:41 +0200
> The Dutch consumer grade ISDN network will be shut down on September 1,
> 2019. This means I'll be converted to some sort of VOIP shortly. At that
> point it would be unwise to try to maintain the gigaset driver, even for
> odd fixes as I do. So I'll stop maintaining it as a seperate driver and
> bump support to CAPI in staging. De facto this means the driver will be
> unmaintained, since no-one seems to be working on CAPI.
>
> I've lighty tested the hardware specific modules of this driver (bas-gigaset,
> ser-gigaset, and usb-gigaset) for v5.3-rc1. The basic functionality appears to
> be working. It's unclear whether anyone still cares. I'm aware of only one
> person sort of using the driver a few years ago.
>
> Thanks to Karsten Keil for the ISDN subsystems gigaset was using (I4L and
> CAPI). And many thanks to Hansjoerg Lipp and Tilman Schmidt for writing and
> upstreaming this driver.
>
> Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
Applied.
^ permalink raw reply
* Re: [PATCH net-next 3/3] net: dsa: mt7530: Add support for port 5
From: René van Dorst @ 2019-07-27 21:26 UTC (permalink / raw)
To: Russell King - ARM Linux admin
Cc: netdev, frank-w, sean.wang, f.fainelli, davem, matthias.bgg,
andrew, vivien.didelot, john, linux-mediatek, linux-mips, robh+dt,
devicetree
In-Reply-To: <20190727185315.GU1330@shell.armlinux.org.uk>
Quoting Russell King - ARM Linux admin <linux@armlinux.org.uk>:
> On Wed, Jul 24, 2019 at 09:25:49PM +0200, René van Dorst wrote:
>> Adding support for port 5.
>>
>> Port 5 can muxed/interface to:
>> - internal 5th GMAC of the switch; can be used as 2nd CPU port or as
>> extra port with an external phy for a 6th ethernet port.
>> - internal PHY of port 0 or 4; Used in most applications so that port 0
>> or 4 is the WAN port and interfaces with the 2nd GMAC of the SOC.
>
> ...
>
>> @@ -1381,15 +1506,19 @@ static void mt7530_phylink_validate(struct
>> dsa_switch *ds, int port,
>> phylink_set_port_modes(mask);
>> phylink_set(mask, Autoneg);
>>
>> - if (state->interface != PHY_INTERFACE_MODE_TRGMII) {
>> + if (state->interface == PHY_INTERFACE_MODE_TRGMII) {
>> + phylink_set(mask, 1000baseT_Full);
>> + } else {
>> phylink_set(mask, 10baseT_Half);
>> phylink_set(mask, 10baseT_Full);
>> phylink_set(mask, 100baseT_Half);
>> phylink_set(mask, 100baseT_Full);
>> - phylink_set(mask, 1000baseT_Half);
>> - }
>>
>> - phylink_set(mask, 1000baseT_Full);
>> + if (state->interface != PHY_INTERFACE_MODE_MII) {
>> + phylink_set(mask, 1000baseT_Half);
>> + phylink_set(mask, 1000baseT_Full);
>> + }
>> + }
>
Hi Russell,
Thanks for your review and many useful comments and explanations.
> As port 5 could use an external PHY, and it supports gigabit speeds,
> consider that the PHY may provide not only copper but also fiber
> connectivity, so port 5 should probably also have 1000baseX modes
> too, which would allow such a PHY to bridge the switch to fiber.
I shall add the 1000baseX modes.
My device, Ubiquiti EdgeRouter X SFP, has this setup.
Port 5 is connected to a at8033 phy which acts as a RGMII-SerDes converter for
the SFP cage. According to the datasheet it only support 100BASE-FX and
1000BASE-X. With bootstrap resistors the PHY is put in RGMII-SerDes 1000BASE-X
mode.
The problem I had is that the current mainline driver doesn't support
this mode
so I had to hack it in myself [0][1]. I probably doing the wrong thing with my
phy driver. Driver works for me, it detects a link and sets-up a 1gbit link.
So I can test port 5. But the driver may report all the wrong values to
PHYLIB/PHYLINK. But now that I learned more about it I can revise the driver.
By reading your previous emails, my setup could official not support the
FiberStore SFP-GB-GE-T module, because it requests a SGMII interface.
But my PHY only supports 1000BaseX and my code currently doesn't error out.
dmesg output of this module:
[ 3.382637] sfp sfp_lan5: module FiberStore SFP-GB-GE-T
rev B sn <snip> dc 19-12-17
[ 3.402048] sfp sfp_lan5: unknown/unspecified connector, encoding
8b10b, nominal bitrate 1.3Gbps +0% -0%
[ 3.421268] sfp sfp_lan5: 1000BaseSX- 1000BaseLX- 1000BaseCX-
1000BaseT+ 100BaseLX- 100BaseFX- BaseBX10- BasePX-
[ 3.441867] sfp sfp_lan5: 10GBaseSR- 10GBaseLR- 10GBaseLRM- 10GBaseER-
[ 3.455208] sfp sfp_lan5: Copper length: 100m
[ 3.464225] sfp sfp_lan5: Options: txdisable
[ 3.473066] sfp sfp_lan5: Diagnostics:
[ 3.481034] sfp sfp_lan5: Unknown/unsupported extended compliance
code: 0x01
[ 3.495069] Atheros 8031 ethernet mdio-bus:07: SFP interface sgmii
What is the best way to do it in case of SGMII interface request?
Return that we don't support SGMII or report that we only support 1
mode and no
auto-negotiation?
Greats,
René
>
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
> According to speedtest.net: 11.9Mbps down 500kbps up
[0]:
https://github.com/vDorst/linux-1/commit/dad5d6ec65cfa99c204e9756b3fc234071709292
[1]:
https://github.com/vDorst/linux-1/commit/a3aa74e84796604ab8619cfaf1c299c115a8736f
^ permalink raw reply
* Re: [PATCH net-next 0/4] r8169: improve HW csum and TSO handling
From: David Miller @ 2019-07-27 21:25 UTC (permalink / raw)
To: hkallweit1; +Cc: nic_swsd, netdev
In-Reply-To: <d347af97-0b46-6c71-37ef-46ce2b46f4df@gmail.com>
From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Fri, 26 Jul 2019 21:47:30 +0200
> This series:
> - delegates more tasks from the driver to the core
> - enables HW csum and TSO per default
> - copies quirks for buggy chip versions from vendor driver
Series applied, thanks Heiner.
^ permalink raw reply
* Re: [PATCH net-next] ipv6: remove printk
From: David Miller @ 2019-07-27 21:24 UTC (permalink / raw)
To: jonathan.lemon; +Cc: netdev, kernel-team
In-Reply-To: <20190726191609.3601197-1-jonathan.lemon@gmail.com>
From: Jonathan Lemon <jonathan.lemon@gmail.com>
Date: Fri, 26 Jul 2019 12:16:09 -0700
> ipv6_find_hdr() prints a non-rate limited error message
> when it cannot find an ipv6 header at a specific offset.
> This could be used as a DoS, so just remove it.
>
> Signed-off-by: Jonathan Lemon <jonathan.lemon@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] r8169: align setting PME with vendor driver
From: David Miller @ 2019-07-27 21:23 UTC (permalink / raw)
To: hkallweit1; +Cc: nic_swsd, netdev
In-Reply-To: <dfc84691-5643-63be-6338-55fe56df18b9@gmail.com>
From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Fri, 26 Jul 2019 20:56:20 +0200
> Align setting PME with the vendor driver. PMEnable is writable on
> RTL8169 only, on later chip versions it's read-only. PME_SIGNAL is
> used on chip versions from RTL8168evl with the exception of the
> RTL8168f family.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next v2] mlx4/en_netdev: allow offloading VXLAN over VLAN
From: David Miller @ 2019-07-27 21:22 UTC (permalink / raw)
To: dcaratti; +Cc: pabeni, marcelo.leitner, saeedm, tariqt, netdev
In-Reply-To: <2beb05557960e04aa588ecc09e9ee5e5a19fc651.1564164688.git.dcaratti@redhat.com>
From: Davide Caratti <dcaratti@redhat.com>
Date: Fri, 26 Jul 2019 20:18:12 +0200
> ConnectX-3 Pro can offload transmission of VLAN packets with VXLAN inside:
> enable tunnel offloads in dev->vlan_features, like it's done with other
> NIC drivers (e.g. be2net and ixgbe).
>
> It's no more necessary to change dev->hw_enc_features when VXLAN are added
> or removed, since .ndo_features_check() already checks for VXLAN packet
> where the UDP destination port matches the configured value. Just set
> dev->hw_enc_features when the NIC is initialized, so that overlying VLAN
> can correctly inherit the tunnel offload capabilities.
>
> Changes since v1:
> - avoid flipping hw_enc_features, instead of calling netdev notifiers,
> thanks to Saeed Mahameed
> - squash two patches into a single one
>
> CC: Paolo Abeni <pabeni@redhat.com>
> CC: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> Signed-off-by: Davide Caratti <dcaratti@redhat.com>
Applied.
^ permalink raw reply
* Re: [PATCH] drivers: net: xgene: Move status variable declaration into CONFIG_ACPI block
From: David Miller @ 2019-07-27 21:18 UTC (permalink / raw)
To: natechancellor
Cc: iyappan, keyur, quan, netdev, linux-kernel, skunberg.kelsey
In-Reply-To: <20190726162037.37308-1-natechancellor@gmail.com>
From: Nathan Chancellor <natechancellor@gmail.com>
Date: Fri, 26 Jul 2019 09:20:37 -0700
> When CONFIG_ACPI is unset (arm allyesconfig), status is unused.
>
> drivers/net/ethernet/apm/xgene/xgene_enet_xgmac.c:383:14: warning:
> unused variable 'status' [-Wunused-variable]
> acpi_status status;
> ^
> drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.c:440:14: warning:
> unused variable 'status' [-Wunused-variable]
> acpi_status status;
> ^
> drivers/net/ethernet/apm/xgene/xgene_enet_hw.c:697:14: warning: unused
> variable 'status' [-Wunused-variable]
> acpi_status status;
> ^
>
> Move the declaration into the CONFIG_ACPI block so that there are no
> compiler warnings.
>
> Fixes: 570d785ba46b ("drivers: net: xgene: Remove acpi_has_method() calls")
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next v2 1/2] qed: Add API for configuring NVM attributes.
From: David Miller @ 2019-07-27 21:00 UTC (permalink / raw)
To: skalluru; +Cc: netdev, mkalderon, aelior
In-Reply-To: <20190726155215.25151-2-skalluru@marvell.com>
From: Sudarsana Reddy Kalluru <skalluru@marvell.com>
Date: Fri, 26 Jul 2019 08:52:14 -0700
> +int qed_mcp_nvm_set_cfg(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt,
> + u16 option_id, u8 entity_id, u16 flags, u8 *p_buf,
> + u32 len)
> +{
> + u32 mb_param = 0, resp, param;
> + int rc;
...
> + rc = qed_mcp_nvm_wr_cmd(p_hwfn, p_ptt,
> + DRV_MSG_CODE_SET_NVM_CFG_OPTION,
> + mb_param, &resp, ¶m, len, (u32 *)p_buf);
> +
> + return rc;
'rc' is completely unnecessary, please just return the function result
directly.
Thank you.
^ permalink raw reply
* Re: [PATCH] net: rds: Fix possible null-pointer dereferences in rds_rdma_cm_event_handler_cmn()
From: David Miller @ 2019-07-27 20:58 UTC (permalink / raw)
To: baijiaju1990
Cc: santosh.shilimkar, netdev, linux-rdma, rds-devel, linux-kernel
In-Reply-To: <20190726141705.9585-1-baijiaju1990@gmail.com>
From: Jia-Ju Bai <baijiaju1990@gmail.com>
Date: Fri, 26 Jul 2019 22:17:05 +0800
> In rds_rdma_cm_event_handler_cmn(), there are some if statements to
> check whether conn is NULL, such as on lines 65, 96 and 112.
> But conn is not checked before being used on line 108:
> trans->cm_connect_complete(conn, event);
> and on lines 140-143:
> rdsdebug("DISCONNECT event - dropping connection "
> "%pI6c->%pI6c\n", &conn->c_laddr,
> &conn->c_faddr);
> rds_conn_drop(conn);
>
> Thus, possible null-pointer dereferences may occur.
>
> To fix these bugs, conn is checked before being used.
>
> These bugs are found by a static analysis tool STCheck written by us.
>
> Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next 2/2] net: stmmac: Do not request stmmaceth clock
From: David Miller @ 2019-07-27 20:52 UTC (permalink / raw)
To: thierry.reding
Cc: joabreu, alexandre.torgue, peppe.cavallaro, jonathanh, netdev,
linux-tegra, linux-arm-kernel
In-Reply-To: <20190726102741.27872-2-thierry.reding@gmail.com>
From: Thierry Reding <thierry.reding@gmail.com>
Date: Fri, 26 Jul 2019 12:27:41 +0200
> From: Thierry Reding <treding@nvidia.com>
>
> The stmmaceth clock is specified by the slave_bus and apb_pclk clocks in
> the device tree bindings for snps,dwc-qos-ethernet-4.10 compatible nodes
> of this IP.
>
> The subdrivers for these bindings will be requesting the stmmac clock
> correctly at a later point, so there is no need to request it here and
> cause an error message to be printed to the kernel log.
>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next 1/2] net: stmmac: Make MDIO bus reset optional
From: David Miller @ 2019-07-27 20:52 UTC (permalink / raw)
To: thierry.reding
Cc: joabreu, alexandre.torgue, peppe.cavallaro, jonathanh, netdev,
linux-tegra, linux-arm-kernel
In-Reply-To: <20190726102741.27872-1-thierry.reding@gmail.com>
From: Thierry Reding <thierry.reding@gmail.com>
Date: Fri, 26 Jul 2019 12:27:40 +0200
> From: Thierry Reding <treding@nvidia.com>
>
> The Tegra EQOS driver already resets the MDIO bus at probe time via the
> reset GPIO specified in the phy-reset-gpios device tree property. There
> is no need to reset the bus again later on.
>
> This avoids the need to query the device tree for the snps,reset GPIO,
> which is not part of the Tegra EQOS device tree bindings. This quiesces
> an error message from the generic bus reset code if it doesn't find the
> snps,reset related delays.
>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
Applied.
^ permalink raw reply
* Re: [PATCH] net: neigh: remove redundant assignment to variable bucket
From: David Miller @ 2019-07-27 20:32 UTC (permalink / raw)
To: colin.king; +Cc: dsahern, netdev, kernel-janitors, linux-kernel
In-Reply-To: <20190726094611.3597-1-colin.king@canonical.com>
From: Colin King <colin.king@canonical.com>
Date: Fri, 26 Jul 2019 10:46:11 +0100
> From: Colin Ian King <colin.king@canonical.com>
>
> The variable bucket is being initialized with a value that is never
> read and it is being updated later with a new value in a following
> for-loop. The initialization is redundant and can be removed.
>
> Addresses-Coverity: ("Unused value")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Applied to net-next.
^ permalink raw reply
* Re: [PATCH] isdn: mISDN: hfcsusb: Fix possible null-pointer dereferences in start_isoc_chain()
From: David Miller @ 2019-07-27 20:29 UTC (permalink / raw)
To: baijiaju1990
Cc: isdn, pakki001, tranmanphong, gregkh, rfontana, gustavo, tglx,
netdev, linux-kernel
In-Reply-To: <20190726082736.8195-1-baijiaju1990@gmail.com>
From: Jia-Ju Bai <baijiaju1990@gmail.com>
Date: Fri, 26 Jul 2019 16:27:36 +0800
> In start_isoc_chain(), usb_alloc_urb() on line 1392 may fail
> and return NULL. At this time, fifo->iso[i].urb is assigned to NULL.
>
> Then, fifo->iso[i].urb is used at some places, such as:
> LINE 1405: fill_isoc_urb(fifo->iso[i].urb, ...)
> urb->number_of_packets = num_packets;
> urb->transfer_flags = URB_ISO_ASAP;
> urb->actual_length = 0;
> urb->interval = interval;
> LINE 1416: fifo->iso[i].urb->...
> LINE 1419: fifo->iso[i].urb->...
>
> Thus, possible null-pointer dereferences may occur.
>
> To fix these bugs, "continue" is added to avoid using fifo->iso[i].urb
> when it is NULL.
>
> These bugs are found by a static analysis tool STCheck written by us.
>
> Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH 2/2] net: ipv6: Fix a possible null-pointer dereference in vti6_link_config()
From: David Miller @ 2019-07-27 20:27 UTC (permalink / raw)
To: baijiaju1990; +Cc: kuznet, yoshfuji, netdev, linux-kernel
In-Reply-To: <20190726080321.4466-1-baijiaju1990@gmail.com>
From: Jia-Ju Bai <baijiaju1990@gmail.com>
Date: Fri, 26 Jul 2019 16:03:21 +0800
> @@ -646,9 +646,10 @@ static void vti6_link_config(struct ip6_tnl *t, bool keep_mtu)
> &p->raddr, &p->laddr,
> p->link, NULL, strict);
>
> - if (rt)
> + if (rt) {
> tdev = rt->dst.dev;
> - ip6_rt_put(rt);
> + ip6_rt_put(rt);
> + }
ip6_rt_put() can take a NULL argument without any problem.
I want to make it clear that because of mistakes of this nature, and
how frequently you make them, it is very tiring and exhausting to
review your changes...
^ permalink raw reply
* Re: [PATCH 1/2] net: ipv4: Fix a possible null-pointer dereference in inet_csk_rebuild_route()
From: David Miller @ 2019-07-27 20:25 UTC (permalink / raw)
To: baijiaju1990; +Cc: kuznet, yoshfuji, netdev, linux-kernel
In-Reply-To: <20190726022534.24994-1-baijiaju1990@gmail.com>
From: Jia-Ju Bai <baijiaju1990@gmail.com>
Date: Fri, 26 Jul 2019 10:25:34 +0800
> diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
> index f5c163d4771b..27d9d80f3401 100644
> --- a/net/ipv4/inet_connection_sock.c
> +++ b/net/ipv4/inet_connection_sock.c
> @@ -1073,7 +1073,10 @@ static struct dst_entry *inet_csk_rebuild_route(struct sock *sk, struct flowi *f
> sk_setup_caps(sk, &rt->dst);
> rcu_read_unlock();
>
> - return &rt->dst;
> + if (rt)
> + return &rt->dst;
> + else
> + return NULL;
If rt is NULL, &rt->dst is also NULL as has been pointed out to you.
Please fix your automated tools to understand this case before submitting
more changes.
Thank you.
^ permalink raw reply
* Re: [PATCH net] mvpp2: refactor MTU change code
From: David Miller @ 2019-07-27 20:24 UTC (permalink / raw)
To: mcroce; +Cc: netdev, antoine.tenart, maxime.chevallier, mw, stefanc,
linux-kernel
In-Reply-To: <20190725231931.24073-1-mcroce@redhat.com>
From: Matteo Croce <mcroce@redhat.com>
Date: Fri, 26 Jul 2019 01:19:31 +0200
> The MTU change code can call napi_disable() with the device already down,
> leading to a deadlock. Also, lot of code is duplicated unnecessarily.
>
> Rework mvpp2_change_mtu() to avoid the deadlock and remove duplicated code.
>
> Signed-off-by: Matteo Croce <mcroce@redhat.com>
Please resubmit with the Fixes: tag.
Please do not line break the Fixes: tag no matter how many characters
it ends up being in total.
Thanks.
^ permalink raw reply
* Re: [PATCH net-next] mvpp2: document HW checksum behaviour
From: David Miller @ 2019-07-27 20:23 UTC (permalink / raw)
To: mcroce; +Cc: antoine.tenart, netdev, maxime.chevallier, mw, stefanc,
linux-kernel
In-Reply-To: <CAGnkfhy_h_UfoefRmBjQgUgiX+954fQjX2kqa2hPLbKpLHU4rg@mail.gmail.com>
From: Matteo Croce <mcroce@redhat.com>
Date: Fri, 26 Jul 2019 16:35:59 +0200
> I see, there is a similar statement in mvpp2_port_probe().
> What about adding a static function which sets the flag, and add the
> comment there instead of duplicating the comment?
That sounds good to me.
^ permalink raw reply
* Re: [PATCH 0/2] Make ipmr queue length configurable
From: David Miller @ 2019-07-27 20:18 UTC (permalink / raw)
To: brodie.greenfield
Cc: stephen, kuznet, yoshfuji, netdev, linux-kernel, chris.packham,
luuk.paulussen
In-Reply-To: <20190725204230.12229-1-brodie.greenfield@alliedtelesis.co.nz>
From: Brodie Greenfield <brodie.greenfield@alliedtelesis.co.nz>
Date: Fri, 26 Jul 2019 08:42:28 +1200
> We want to have some more space in our queue for processing incoming
> multicast packets, so we can process more of them without dropping
> them prematurely. It is useful to be able to increase this limit on
> higher-spec platforms that can handle more items.
>
> For the particular use case here at Allied Telesis, we have linux
> running on our switches and routers, with support for the number of
> multicast groups being increased. Basically, this queue length affects
> the time taken to fully learn all of the multicast streams.
>
> Changes in v3:
> - Corrected a v4 to v6 typo.
As others have voiced, I think it's dangerous to let every netns
increase this so readily.
We need to either put in a non-initns limit or simply not allow
non-init namespaces to change this.
But really socket queue limits are a better place to enforce this.
^ permalink raw reply
* Re: [PATCH net-next] net: dsa: mv88e6xxx: avoid some redundant vtu load/purge operations
From: David Miller @ 2019-07-27 20:14 UTC (permalink / raw)
To: rasmus.villemoes
Cc: andrew, vivien.didelot, f.fainelli, Rasmus.Villemoes, netdev,
linux-kernel
In-Reply-To: <20190722233713.31396-1-rasmus.villemoes@prevas.dk>
Reminder that we need a review from Vivien on this.
Thanks.
^ permalink raw reply
* [PATCH] net: stmmac: manage errors returned by of_get_mac_address()
From: Martin Blumenstingl @ 2019-07-27 19:21 UTC (permalink / raw)
To: peppe.cavallaro, alexandre.torgue, joabreu, davem, netdev
Cc: linux-arm-kernel, linux-kernel, linux-amlogic,
Martin Blumenstingl
Commit d01f449c008a ("of_net: add NVMEM support to of_get_mac_address")
added support for reading the MAC address from an nvmem-cell. This
required changing the logic to return an error pointer upon failure.
If stmmac is loaded before the nvmem provider driver then
of_get_mac_address() return an error pointer with -EPROBE_DEFER.
Propagate this error so the stmmac driver will be probed again after the
nvmem provider driver is loaded.
Default to a random generated MAC address in case of any other error,
instead of using the error pointer as MAC address.
Fixes: d01f449c008a ("of_net: add NVMEM support to of_get_mac_address")
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index 73fc2524372e..154daf4d1072 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -370,6 +370,13 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
return ERR_PTR(-ENOMEM);
*mac = of_get_mac_address(np);
+ if (IS_ERR(*mac)) {
+ if (PTR_ERR(*mac) == -EPROBE_DEFER)
+ return ERR_CAST(*mac);
+
+ *mac = NULL;
+ }
+
plat->interface = of_get_phy_mode(np);
/* Some wrapper drivers still rely on phy_node. Let's save it while
--
2.22.0
^ permalink raw reply related
* Re: [PATCH bpf-next 02/10] libbpf: implement BPF CO-RE offset relocation algorithm
From: Andrii Nakryiko @ 2019-07-27 19:09 UTC (permalink / raw)
To: Song Liu
Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov,
Daniel Borkmann, Yonghong Song, Kernel Team
In-Reply-To: <9EE75932-5AED-49D3-86BF-D1FC2A139BF8@fb.com>
On Sat, Jul 27, 2019 at 11:59 AM Song Liu <songliubraving@fb.com> wrote:
>
>
>
> > On Jul 26, 2019, at 11:11 PM, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
> >
> > On Thu, Jul 25, 2019 at 12:32 PM Song Liu <songliubraving@fb.com> wrote:
> >>
> >>
> >>
> >>> On Jul 24, 2019, at 12:27 PM, Andrii Nakryiko <andriin@fb.com> wrote:
> >>>
> >>> This patch implements the core logic for BPF CO-RE offsets relocations.
> >>> All the details are described in code comments.
> >>
> >> Some description in the change log is still useful. Please at least
> >> copy-paste key comments here.
> >
> > OK, will add some more.
> >
> >>
> >> And, this is looooong. I think it is totally possible to split it into
> >> multiple smaller patches.
> >
> > I don't really know how to split it further without hurting reviewing
> > by artificially splitting related code into separate patches. Remove
> > any single function and algorithm will be incomplete.
> >
> > Let me give you some high-level overview of how pieces are put
> > together. There are 9 non-trivial functions, let's go over their
> > purpose in the orderd in which they are defined in file:
> >
> > 1. bpf_core_spec_parse()
> >
> > This one take bpf_offset_reloc's type_id and accessor string
> > ("0:1:2:3") and parses it into more convenient bpf_core_spec
> > datastructure, which has calculated offset and high-level spec
> > "steps": either named field or array access.
> >
> > 2. bpf_core_find_cands()
> >
> > Given local type name, finds all possible target BTF types with same
> > name (modulo "flavor" differences, ___flavor suffix is just ignored).
> >
> > 3. bpf_core_fields_are_compat()
> >
> > Given local and target field match, checks that their types are
> > compatible (so that we don't accidentally match, e.g., int against
> > struct).
> >
> > 4. bpf_core_match_member()
> >
> > Given named local field, find corresponding field in target struct. To
> > understand why it's not trivial, here's an example:
> >
> > Local type:
> >
> > struct s___local {
> > int a;
> > };
> >
> > Target type:
> >
> > struct s___target {
> > struct {
> > union {
> > int a;
> > };
> > };
> > };
> >
> > For both cases you can access a as s.a, but in local case, field a is
> > immediately inside s___local, while for s___target, you have to
> > traverse two levels deeper into anonymous fields to get to an `a`
> > inside anonymous union.
> >
> > So this function find that `a` by doing exhaustive search across all
> > named field and anonymous struct/unions. But otherwise it's pretty
> > straightforward recursive function.
> >
> > bpf_core_spec_match()
> >
> > Just goes over high-level spec steps in local spec and tries to figure
> > out both high-level and low-level steps for targe type. Consider the
> > above example. For both structs accessing s.a is one high-level step,
> > but for s___local it's single low-level step (just another :0 in spec
> > string), while for s___target it's three low-level steps: ":0:0:0",
> > one step for each BTF type we need to traverse.
> >
> > Array access is simpler, it's always one high-level and one low-level step.
> >
> > bpf_core_reloc_insn()
> >
> > Once we match local and target specs and have local and target
> > offsets, do the relocations - check that instruction has expected
> > local offset and replace it with target offset.
> >
> > bpf_core_find_kernel_btf()
> >
> > This is the only function that can be moved into separate patch, but
> > it's also very simple. It just iterates over few known possible
> > locations for vmlinux image and once found, tries to parse .BTF out of
> > it, to be used as target BTF.
> >
> > bpf_core_reloc_offset()
> >
> > It combines all the above functions to perform single relocation.
> > Parse spec, get candidates, for each candidate try to find matching
> > target spec. All candidates that matched are cached for given local
> > root type.
>
> Thanks for these explanation. They are really helpful.
>
> I think some example explaining each step of bpf_core_reloc_offset()
> will be very helpful. Something like:
>
> Example:
>
> struct s {
> int a;
> struct {
> int b;
> bool c;
> };
> };
>
> To get offset for c, we do:
>
> bpf_core_reloc_offset() {
>
> /* input data: xxx */
>
> /* first step: bpf_core_spec_parse() */
>
> /* data after first step */
>
> /* second step: bpf_core_find_cands() */
>
> /* candidate A and B after second step */
>
> ...
> }
>
> Well, it requires quite some work to document this way. Please let me
> know if you feel this is an overkill.
Yeah :) And it's not just work, but I think it's bad if comments
become too specific and document very low-level steps, because code
might evolve and comments can quickly get out of sync and just add to
confusion. Which is why I tried to document high-level ideas, leaving
it up to the source code to be the ultimate reference of minutia
details.
>
> >
> > bpf_core_reloc_offsets()
> >
> > High-level coordination. Iterate over all per-program .BTF.ext offset
> > reloc sections, each relocation within them. Find corresponding
> > program and try to apply relocations one by one.
> >
> >
> > I think the only non-obvious part here is to understand that
> > relocation records local raw spec with every single anonymous type
> > traversal, which is not that useful when we try to match it against
> > target type, which can have very different composition, but still the
> > same field access pattern, from C language standpoint (which hides all
> > those anonymous type traversals from programmer).
> >
> > But it should be pretty clear now, plus also check tests, they have
> > lots of cases showing what's compatible and what's not.
>
> I see. I will review the tests.
>
> >>>
> >>> static const struct btf_type *skip_mods_and_typedefs(const struct btf *btf,
> >>> - __u32 id)
> >>> + __u32 id,
> >>> + __u32 *res_id)
> >>> {
> >>> const struct btf_type *t = btf__type_by_id(btf, id);
> >>
> >> Maybe have a local "__u32 rid;"
> >>
> >>>
> >>> + if (res_id)
> >>> + *res_id = id;
> >>> +
> >>
> >> and do "rid = id;" here
> >>
> >>> while (true) {
> >>> switch (BTF_INFO_KIND(t->info)) {
> >>> case BTF_KIND_VOLATILE:
> >>> case BTF_KIND_CONST:
> >>> case BTF_KIND_RESTRICT:
> >>> case BTF_KIND_TYPEDEF:
> >>> + if (res_id)
> >>> + *res_id = t->type;
> >> and here
> >>
> >>> t = btf__type_by_id(btf, t->type);
> >>> break;
> >>> default:
> >> and "*res_id = rid;" right before return?
> >
> > Sure, but why?
>
> I think it is cleaner that way. But feel free to ignore if you
> think otherwise.
>
> >
> >>
> >>> @@ -1041,7 +1049,7 @@ static const struct btf_type *skip_mods_and_typedefs(const struct btf *btf,
> >>> static bool get_map_field_int(const char *map_name, const struct btf *btf,
> >>> const struct btf_type *def,
> >>> const struct btf_member *m, __u32 *res) {
> >
> > [...]
> >
> >>> +struct bpf_core_spec {
> >>> + const struct btf *btf;
> >>> + /* high-level spec: named fields and array indicies only */
> >>
> >> typo: indices
> >
> > thanks!
> >
> >>
> >>> + struct bpf_core_accessor spec[BPF_CORE_SPEC_MAX_LEN];
> >>> + /* high-level spec length */
> >>> + int len;
> >>> + /* raw, low-level spec: 1-to-1 with accessor spec string */
> >>> + int raw_spec[BPF_CORE_SPEC_MAX_LEN];
> >>> + /* raw spec length */
> >>> + int raw_len;
> >>> + /* field byte offset represented by spec */
> >>> + __u32 offset;
> >>> +};
> >
> > [...]
> >
> >>> + *
> >>> + * int x = &s->a[3]; // access string = '0:1:2:3'
> >>> + *
> >>> + * Low-level spec has 1:1 mapping with each element of access string (it's
> >>> + * just a parsed access string representation): [0, 1, 2, 3].
> >>> + *
> >>> + * High-level spec will capture only 3 points:
> >>> + * - intial zero-index access by pointer (&s->... is the same as &s[0]...);
> >>> + * - field 'a' access (corresponds to '2' in low-level spec);
> >>> + * - array element #3 access (corresponds to '3' in low-level spec).
> >>> + *
> >>> + */
> >>
> >> IIUC, high-level points are subset of low-level points. How about we introduce
> >> "anonymous" high-level points, so that high-level points and low-level points
> >> are 1:1 mapping?
> >
> > No, that will just hurt and complicate things. See above explanation
> > about why we need high-level points (it's what you as C programmer try
> > to achieve vs low-level spec is what C-language does in reality, with
> > all the anonymous struct/union traversal).
> >
> > What's wrong with this separation? Think about it as recording
> > "intent" (high-level spec) vs "mechanics" (low-level spec, how exactly
> > to achieve that intent, in excruciating details).
>
> There is nothing wrong with separation. I just personally think it is
> cleaner the other way. That's why I raised the question.
>
> I will go with your assessment, as you looked into this much more than
> I did. :-)
For me it's a machine view of the problem (raw spec) vs human view of
the problem (high-level spec, which resembles how you think about this
in C code). I'll keep it separate unless it proves to be problematic
going forward.
>
> [...]
>
> >>> +
> >>> + memset(spec, 0, sizeof(*spec));
> >>> + spec->btf = btf;
> >>> +
> >>> + /* parse spec_str="0:1:2:3:4" into array raw_spec=[0, 1, 2, 3, 4] */
> >>> + while (*spec_str) {
> >>> + if (*spec_str == ':')
> >>> + ++spec_str;
> >>> + if (sscanf(spec_str, "%d%n", &access_idx, &parsed_len) != 1)
> >>> + return -EINVAL;
> >>> + if (spec->raw_len == BPF_CORE_SPEC_MAX_LEN)
> >>> + return -E2BIG;
> >>> + spec_str += parsed_len;
> >>> + spec->raw_spec[spec->raw_len++] = access_idx;
> >>> + }
> >>> +
> >>> + if (spec->raw_len == 0)
> >>> + return -EINVAL;
> >>> +
> >>> + for (i = 0; i < spec->raw_len; i++) {
> >>> + t = skip_mods_and_typedefs(btf, id, &id);
> >>> + if (!t)
> >>> + return -EINVAL;
> >>> +
> >>> + access_idx = spec->raw_spec[i];
> >>> +
> >>> + if (i == 0) {
> >>> + /* first spec value is always reloc type array index */
> >>> + spec->spec[spec->len].type_id = id;
> >>> + spec->spec[spec->len].idx = access_idx;
> >>> + spec->len++;
> >>> +
> >>> + sz = btf__resolve_size(btf, id);
> >>> + if (sz < 0)
> >>> + return sz;
> >>> + spec->offset += access_idx * sz;
> >> spec->offset = access_idx * sz; should be enough
> >
> > No. spec->offset is carefully maintained across multiple low-level
> > steps, as we traverse down embedded structs/unions.
> >
> > Think about, e.g.:
> >
> > struct s {
> > int a;
> > struct {
> > int b;
> > };
> > };
> >
> > Imagine you are trying to match s.b access. With what you propose
> > you'll end up with offset 0, but it should be 4.
>
> Hmm... this is just for i == 0, right? Which line updated spec->offset
> after "memset(spec, 0, sizeof(*spec));"?
Ah, I missed that you are referring to the special i == 0 case. I can
do assignment, yes, you are right. I'll probably also extract it out
of the loop to make it less confusing.
>
> >
> >>
> >>> + continue;
> >>> + }
> >>
> >> Maybe pull i == 0 case out of the for loop?
> >>
> >>> +
> >>> + if (btf_is_composite(t)) {
> >
> > [...]
> >
> >>> +
> >>> + if (spec->len == 0)
> >>> + return -EINVAL;
> >>
> >> Can this ever happen?
> >
> > Not really, because I already check raw_len == 0 and exit with error.
> > I'll remove.
> >
> >>
> >>> +
> >>> + return 0;
> >>> +}
> >>> +
> >
> > [...]
> >
> >>> +
> >>> +/*
> >>> + * Given single high-level accessor (either named field or array index) in
> >>> + * local type, find corresponding high-level accessor for a target type. Along
> >>> + * the way, maintain low-level spec for target as well. Also keep updating
> >>> + * target offset.
> >>> + */
> >>
> >> Please describe the recursive algorithm here. I am kinda lost.
> >
> > Explained above. I'll extend description a bit. But it's just
> > recursive exhaustive search:
> > 1. if struct field is anonymous and is struct/union, go one level
> > deeper and try to find field with given name inside those.
> > 2. if field has name and it matched what we are searching - check type
> > compatibility. It has to be compatible, so if it's not, then it's not
> > a match.
> >
> >> Also, please document the meaning of zero, positive, negative return values.
> >
> > Ok. It's standard <0 - error, 0 - false, 1 - true.
> >
> >>
> >>> +static int bpf_core_match_member(const struct btf *local_btf,
> >>> + const struct bpf_core_accessor *local_acc,
> >>> + const struct btf *targ_btf,
> >>> + __u32 targ_id,
> >>> + struct bpf_core_spec *spec,
> >>> + __u32 *next_targ_id)
> >>> +{
> >
> > [...]
> >
> >>> + if (local_acc->name) {
> >>> + if (!btf_is_composite(targ_type))
> >>> + return 0;
> >>> +
> >>> + matched = bpf_core_match_member(local_spec->btf,
> >>> + local_acc,
> >>> + targ_btf, targ_id,
> >>> + targ_spec, &targ_id);
> >>> + if (matched <= 0)
> >>> + return matched;
> >>> + } else {
> >>> + /* for i=0, targ_id is already treated as array element
> >>> + * type (because it's the original struct), for others
> >>> + * we should find array element type first
> >>> + */
> >>> + if (i > 0) {
> >>
> >> i == 0 case would go into "if (local_acc->name)" branch, no?
> >
> > No, i == 0 is always an array access. s->a.b.c is the same as
> > s[0].a.b.c, so relocation's first spec element is always either zero
> > for pointer access or any non-negative index for array access. But it
> > is always array access.
>
> I see. Thanks for the explanation.
>
> Song
^ permalink raw reply
* [PATCH v2 bpf-next 8/9] selftests/bpf: convert bpf_verif_scale.c to sub-tests API
From: Andrii Nakryiko @ 2019-07-27 19:01 UTC (permalink / raw)
To: bpf, netdev, ast, daniel, sdf
Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko
In-Reply-To: <20190727190150.649137-1-andriin@fb.com>
Expose each BPF verifier scale test as individual sub-test to allow
independent results output and test selection.
Test run results now look like this:
$ sudo ./test_progs -t verif/
#3/1 loop3.o:OK
#3/2 test_verif_scale1.o:OK
#3/3 test_verif_scale2.o:OK
#3/4 test_verif_scale3.o:OK
#3/5 pyperf50.o:OK
#3/6 pyperf100.o:OK
#3/7 pyperf180.o:OK
#3/8 pyperf600.o:OK
#3/9 pyperf600_nounroll.o:OK
#3/10 loop1.o:OK
#3/11 loop2.o:OK
#3/12 strobemeta.o:OK
#3/13 strobemeta_nounroll1.o:OK
#3/14 strobemeta_nounroll2.o:OK
#3/15 test_sysctl_loop1.o:OK
#3/16 test_sysctl_loop2.o:OK
#3/17 test_xdp_loop.o:OK
#3/18 test_seg6_loop.o:OK
#3 bpf_verif_scale:OK
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
.../bpf/prog_tests/bpf_verif_scale.c | 77 ++++++++++---------
1 file changed, 40 insertions(+), 37 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c b/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c
index b59017279e0b..b4be96162ff4 100644
--- a/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c
@@ -33,14 +33,25 @@ static int check_load(const char *file, enum bpf_prog_type type)
return err;
}
+struct scale_test_def {
+ const char *file;
+ enum bpf_prog_type attach_type;
+ bool fails;
+};
+
void test_bpf_verif_scale(void)
{
- const char *sched_cls[] = {
- "./test_verif_scale1.o", "./test_verif_scale2.o", "./test_verif_scale3.o",
- };
- const char *raw_tp[] = {
+ struct scale_test_def tests[] = {
+ { "loop3.o", BPF_PROG_TYPE_RAW_TRACEPOINT, true /* fails */ },
+
+ { "test_verif_scale1.o", BPF_PROG_TYPE_SCHED_CLS },
+ { "test_verif_scale2.o", BPF_PROG_TYPE_SCHED_CLS },
+ { "test_verif_scale3.o", BPF_PROG_TYPE_SCHED_CLS },
+
/* full unroll by llvm */
- "./pyperf50.o", "./pyperf100.o", "./pyperf180.o",
+ { "pyperf50.o", BPF_PROG_TYPE_RAW_TRACEPOINT },
+ { "pyperf100.o", BPF_PROG_TYPE_RAW_TRACEPOINT },
+ { "pyperf180.o", BPF_PROG_TYPE_RAW_TRACEPOINT },
/* partial unroll. llvm will unroll loop ~150 times.
* C loop count -> 600.
@@ -48,7 +59,7 @@ void test_bpf_verif_scale(void)
* 16k insns in loop body.
* Total of 5 such loops. Total program size ~82k insns.
*/
- "./pyperf600.o",
+ { "pyperf600.o", BPF_PROG_TYPE_RAW_TRACEPOINT },
/* no unroll at all.
* C loop count -> 600.
@@ -56,22 +67,26 @@ void test_bpf_verif_scale(void)
* ~110 insns in loop body.
* Total of 5 such loops. Total program size ~1500 insns.
*/
- "./pyperf600_nounroll.o",
+ { "pyperf600_nounroll.o", BPF_PROG_TYPE_RAW_TRACEPOINT },
- "./loop1.o", "./loop2.o",
+ { "loop1.o", BPF_PROG_TYPE_RAW_TRACEPOINT },
+ { "loop2.o", BPF_PROG_TYPE_RAW_TRACEPOINT },
/* partial unroll. 19k insn in a loop.
* Total program size 20.8k insn.
* ~350k processed_insns
*/
- "./strobemeta.o",
+ { "strobemeta.o", BPF_PROG_TYPE_RAW_TRACEPOINT },
/* no unroll, tiny loops */
- "./strobemeta_nounroll1.o",
- "./strobemeta_nounroll2.o",
- };
- const char *cg_sysctl[] = {
- "./test_sysctl_loop1.o", "./test_sysctl_loop2.o",
+ { "strobemeta_nounroll1.o", BPF_PROG_TYPE_RAW_TRACEPOINT },
+ { "strobemeta_nounroll2.o", BPF_PROG_TYPE_RAW_TRACEPOINT },
+
+ { "test_sysctl_loop1.o", BPF_PROG_TYPE_CGROUP_SYSCTL },
+ { "test_sysctl_loop2.o", BPF_PROG_TYPE_CGROUP_SYSCTL },
+
+ { "test_xdp_loop.o", BPF_PROG_TYPE_XDP },
+ { "test_seg6_loop.o", BPF_PROG_TYPE_LWT_SEG6LOCAL },
};
libbpf_print_fn_t old_print_fn = NULL;
int err, i;
@@ -81,33 +96,21 @@ void test_bpf_verif_scale(void)
old_print_fn = libbpf_set_print(libbpf_debug_print);
}
- err = check_load("./loop3.o", BPF_PROG_TYPE_RAW_TRACEPOINT);
- test__printf("test_scale:loop3:%s\n",
- err ? (error_cnt--, "OK") : "FAIL");
+ for (i = 0; i < ARRAY_SIZE(tests); i++) {
+ const struct scale_test_def *test = &tests[i];
- for (i = 0; i < ARRAY_SIZE(sched_cls); i++) {
- err = check_load(sched_cls[i], BPF_PROG_TYPE_SCHED_CLS);
- test__printf("test_scale:%s:%s\n", sched_cls[i],
- err ? "FAIL" : "OK");
- }
+ if (!test__start_subtest(test->file))
+ continue;
- for (i = 0; i < ARRAY_SIZE(raw_tp); i++) {
- err = check_load(raw_tp[i], BPF_PROG_TYPE_RAW_TRACEPOINT);
- test__printf("test_scale:%s:%s\n", raw_tp[i],
- err ? "FAIL" : "OK");
+ err = check_load(test->file, test->attach_type);
+ if (test->fails) { /* expected to fail */
+ if (err)
+ error_cnt--;
+ else
+ error_cnt++;
+ }
}
- for (i = 0; i < ARRAY_SIZE(cg_sysctl); i++) {
- err = check_load(cg_sysctl[i], BPF_PROG_TYPE_CGROUP_SYSCTL);
- test__printf("test_scale:%s:%s\n", cg_sysctl[i],
- err ? "FAIL" : "OK");
- }
- err = check_load("./test_xdp_loop.o", BPF_PROG_TYPE_XDP);
- test__printf("test_scale:test_xdp_loop:%s\n", err ? "FAIL" : "OK");
-
- err = check_load("./test_seg6_loop.o", BPF_PROG_TYPE_LWT_SEG6LOCAL);
- test__printf("test_scale:test_seg6_loop:%s\n", err ? "FAIL" : "OK");
-
if (env.verifier_stats)
libbpf_set_print(old_print_fn);
}
--
2.17.1
^ permalink raw reply related
* [PATCH v2 bpf-next 9/9] selftests/bpf: convert send_signal.c to use subtests
From: Andrii Nakryiko @ 2019-07-27 19:01 UTC (permalink / raw)
To: bpf, netdev, ast, daniel, sdf
Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko
In-Reply-To: <20190727190150.649137-1-andriin@fb.com>
Convert send_signal set of tests to be exposed as three sub-tests.
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
tools/testing/selftests/bpf/prog_tests/send_signal.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/send_signal.c b/tools/testing/selftests/bpf/prog_tests/send_signal.c
index d950f4558897..461b423d0584 100644
--- a/tools/testing/selftests/bpf/prog_tests/send_signal.c
+++ b/tools/testing/selftests/bpf/prog_tests/send_signal.c
@@ -219,7 +219,10 @@ void test_send_signal(void)
{
int ret = 0;
- ret |= test_send_signal_tracepoint();
- ret |= test_send_signal_perf();
- ret |= test_send_signal_nmi();
+ if (test__start_subtest("send_signal_tracepoint"))
+ ret |= test_send_signal_tracepoint();
+ if (test__start_subtest("send_signal_perf"))
+ ret |= test_send_signal_perf();
+ if (test__start_subtest("send_signal_nmi"))
+ ret |= test_send_signal_nmi();
}
--
2.17.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox