* Re: [PATCH 13/16] include/asm-generic: prefer __section from compiler_attributes.h
From: Naveen N. Rao @ 2019-08-19 17:52 UTC (permalink / raw)
To: akpm, Nick Desaulniers
Cc: Anil S Keshavamurthy, Arnd Bergmann, Alexei Starovoitov, bpf,
clang-built-linux, Daniel Borkmann, David S. Miller, jpoimboe,
Martin KaFai Lau, linux-arch, linux-kernel, Masami Hiramatsu,
miguel.ojeda.sandonis, netdev, sedat.dilek, Song Liu, yhs
In-Reply-To: <20190812215052.71840-13-ndesaulniers@google.com>
Nick Desaulniers wrote:
> Reported-by: Sedat Dilek <sedat.dilek@gmail.com>
> Suggested-by: Josh Poimboeuf <jpoimboe@redhat.com>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
Acked-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
- Naveen
^ permalink raw reply
* [PATCH net-next 0/1] Add genphy_c45_config_aneg() function to phy-c45.c
From: Marco Hartmann @ 2019-08-19 17:52 UTC (permalink / raw)
To: Marco Hartmann, Christian Herber, andrew@lunn.ch,
f.fainelli@gmail.com, hkallweit1@gmail.com, davem@davemloft.net,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Currently, using a Clause 45 Phy with the "Generic Clause 45 PHY"
driver leads to a warning, similar to the one below, as soon as the interface
is brought up.
------------[ cut here ]------------
WARNING: CPU: 2 PID: 146 at drivers/net/phy/phy.c:736 phy_error+0x2c/0x64
Modules linked in: fec
CPU: 2 PID: 146 Comm: kworker/2:1 Not tainted 5.3.0-rc3-NETNEXT-00816-g48e924c73178 #20
Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
Workqueue: events_power_efficient phy_state_machine
Backtrace:
...
This happens, because the Genphy driver does not provide a config_aneg() func,
so that phy_start_aneg() ultimately fails such that phy_error() is called,
producing the above warning.
This patch adds the function genphy_c45_config_aneg(), which allows
phy_start_aneg() to work correctly for C45 phys.
Marco Hartmann (1):
Add genphy_c45_config_aneg() function to phy-c45.c
drivers/net/phy/phy-c45.c | 26 ++++++++++++++++++++++++++
drivers/net/phy/phy.c | 2 +-
include/linux/phy.h | 1 +
3 files changed, 28 insertions(+), 1 deletion(-)
--
2.7.4
^ permalink raw reply
* [PATCH net-next 1/1] Add genphy_c45_config_aneg() function to phy-c45.c
From: Marco Hartmann @ 2019-08-19 17:52 UTC (permalink / raw)
To: Marco Hartmann, Christian Herber, andrew@lunn.ch,
f.fainelli@gmail.com, hkallweit1@gmail.com, davem@davemloft.net,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <1566237157-9054-1-git-send-email-marco.hartmann@nxp.com>
and call it from phy_config_aneg().
commit 34786005eca3 ("net: phy: prevent PHYs w/o Clause 22 regs from
calling genphy_config_aneg") introduced a check that aborts
phy_config_aneg() if the phy is a C45 phy.
This causes phy_state_machine() to call phy_error() so that the phy
ends up in PHY_HALTED state.
Instead of returning -EOPNOTSUPP, call genphy_c45_config_aneg()
(analogous to the C22 case) so that the state machine can run
correctly.
genphy_c45_config_aneg() closely resembles mv3310_config_aneg()
in drivers/net/phy/marvell10g.c, excluding vendor specific
configurations for 1000BaseT.
Fixes: 34786005eca3 ("net: phy: prevent PHYs w/o Clause 22 regs from
calling genphy_config_aneg")
Signed-off-by: Marco Hartmann <marco.hartmann@nxp.com>
---
drivers/net/phy/phy-c45.c | 26 ++++++++++++++++++++++++++
drivers/net/phy/phy.c | 2 +-
include/linux/phy.h | 1 +
3 files changed, 28 insertions(+), 1 deletion(-)
diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
index b9d4145781ca..fa9062fd9122 100644
--- a/drivers/net/phy/phy-c45.c
+++ b/drivers/net/phy/phy-c45.c
@@ -509,6 +509,32 @@ int genphy_c45_read_status(struct phy_device *phydev)
}
EXPORT_SYMBOL_GPL(genphy_c45_read_status);
+/**
+ * genphy_c45_config_aneg - restart auto-negotiation or forced setup
+ * @phydev: target phy_device struct
+ *
+ * Description: If auto-negotiation is enabled, we configure the
+ * advertising, and then restart auto-negotiation. If it is not
+ * enabled, then we force a configuration.
+ */
+int genphy_c45_config_aneg(struct phy_device *phydev)
+{
+ int ret;
+ bool changed = false;
+
+ if (phydev->autoneg == AUTONEG_DISABLE)
+ return genphy_c45_pma_setup_forced(phydev);
+
+ ret = genphy_c45_an_config_aneg(phydev);
+ if (ret < 0)
+ return ret;
+ if (ret > 0)
+ changed = true;
+
+ return genphy_c45_check_and_restart_aneg(phydev, changed);
+}
+EXPORT_SYMBOL_GPL(genphy_c45_config_aneg);
+
/* The gen10g_* functions are the old Clause 45 stub */
int gen10g_config_aneg(struct phy_device *phydev)
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index f3adea9ef400..74c4e15ebe52 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -507,7 +507,7 @@ static int phy_config_aneg(struct phy_device *phydev)
* allowed to call genphy_config_aneg()
*/
if (phydev->is_c45 && !(phydev->c45_ids.devices_in_package & BIT(0)))
- return -EOPNOTSUPP;
+ return genphy_c45_config_aneg(phydev);
return genphy_config_aneg(phydev);
}
diff --git a/include/linux/phy.h b/include/linux/phy.h
index d26779f1fb6b..a7ecbe0e55aa 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -1117,6 +1117,7 @@ int genphy_c45_an_disable_aneg(struct phy_device *phydev);
int genphy_c45_read_mdix(struct phy_device *phydev);
int genphy_c45_pma_read_abilities(struct phy_device *phydev);
int genphy_c45_read_status(struct phy_device *phydev);
+int genphy_c45_config_aneg(struct phy_device *phydev);
/* The gen10g_* functions are the old Clause 45 stub */
int gen10g_config_aneg(struct phy_device *phydev);
--
2.7.4
^ permalink raw reply related
* Re: [PATCH 13/16] include/asm-generic: prefer __section from compiler_attributes.h
From: Sedat Dilek @ 2019-08-19 17:56 UTC (permalink / raw)
To: Naveen N. Rao
Cc: akpm, Nick Desaulniers, Anil S Keshavamurthy, Arnd Bergmann,
Alexei Starovoitov, bpf, Clang-Built-Linux ML, Daniel Borkmann,
David S. Miller, jpoimboe, Martin KaFai Lau, linux-arch,
linux-kernel, Masami Hiramatsu, Miguel Ojeda, netdev, Song Liu,
yhs
In-Reply-To: <1566237106.8670clhnrk.naveen@linux.ibm.com>
On Mon, Aug 19, 2019 at 7:52 PM Naveen N. Rao
<naveen.n.rao@linux.ibm.com> wrote:
>
> Nick Desaulniers wrote:
> > Reported-by: Sedat Dilek <sedat.dilek@gmail.com>
> > Suggested-by: Josh Poimboeuf <jpoimboe@redhat.com>
> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> > ---
>
> Acked-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
>
Tested-by: Sedat Dilek <sedat.dilek@gmail.com> [ Linux v5.3-rc5 ]
Patchset "for-5.3/x86-section-name-escaping":
include/linux/compiler.h: remove unused KENTRY macro
include/linux: prefer __section from compiler_attributes.h
include/asm-generic: prefer __section from compiler_attributes.h
x86: prefer __section from compiler_attributes.h
- Sedat -
^ permalink raw reply
* Re: [PATCH net-next 0/8] sctp: support per endpoint auth and asconf flags
From: Marcelo Ricardo Leitner @ 2019-08-19 17:58 UTC (permalink / raw)
To: Xin Long; +Cc: network dev, linux-sctp, Neil Horman, davem
In-Reply-To: <cover.1566223325.git.lucien.xin@gmail.com>
On Mon, Aug 19, 2019 at 10:02:42PM +0800, Xin Long wrote:
> This patchset mostly does 3 things:
>
> 1. add per endpint asconf flag and use asconf flag properly
> and add SCTP_ASCONF_SUPPORTED sockopt.
> 2. use auth flag properly and add SCTP_AUTH_SUPPORTED sockopt.
> 3. remove the 'global feature switch' to discard chunks.
What about net->sctp.addip_noauth, why wasn't it included as well?
I'm thinking that's because it's more of a compatibility knob and
that there isn't much value on having it more fine-grained than
per-netns. Just feels a bit odd to have the other two but not this
one, which is directly related, but yeah.
Otherwise, changes LGTM.
^ permalink raw reply
* Re: [PATCH 11/16] x86: prefer __section from compiler_attributes.h
From: Sedat Dilek @ 2019-08-19 17:59 UTC (permalink / raw)
To: Nick Desaulniers
Cc: akpm, jpoimboe, yhs, Miguel Ojeda, Clang-Built-Linux ML,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin,
x86, Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau,
Song Liu, Armijn Hemel, Greg Kroah-Hartman, Allison Randal,
Juergen Gross, Frederic Weisbecker, Brijesh Singh, Enrico Weigelt,
Kate Stewart, Hannes Reinecke, Sean Christopherson,
Rafael J. Wysocki, Pu Wen, linux-kernel, netdev, bpf
In-Reply-To: <20190812215052.71840-11-ndesaulniers@google.com>
On Mon, Aug 12, 2019 at 11:52 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> Reported-by: Sedat Dilek <sedat.dilek@gmail.com>
> Suggested-by: Josh Poimboeuf <jpoimboe@redhat.com>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
Tested-by: Sedat Dilek <sedat.dilek@gmail.com> [ Linux v5.3-rc5 ]
Patchset "for-5.3/x86-section-name-escaping":
include/linux/compiler.h: remove unused KENTRY macro
include/linux: prefer __section from compiler_attributes.h
include/asm-generic: prefer __section from compiler_attributes.h
x86: prefer __section from compiler_attributes.h
Thanks.
- Sedat -
> ---
> arch/x86/include/asm/cache.h | 2 +-
> arch/x86/include/asm/intel-mid.h | 2 +-
> arch/x86/include/asm/iommu_table.h | 5 ++---
> arch/x86/include/asm/irqflags.h | 2 +-
> arch/x86/include/asm/mem_encrypt.h | 2 +-
> arch/x86/kernel/cpu/cpu.h | 3 +--
> 6 files changed, 7 insertions(+), 9 deletions(-)
>
> diff --git a/arch/x86/include/asm/cache.h b/arch/x86/include/asm/cache.h
> index abe08690a887..bb9f4bf4ec02 100644
> --- a/arch/x86/include/asm/cache.h
> +++ b/arch/x86/include/asm/cache.h
> @@ -8,7 +8,7 @@
> #define L1_CACHE_SHIFT (CONFIG_X86_L1_CACHE_SHIFT)
> #define L1_CACHE_BYTES (1 << L1_CACHE_SHIFT)
>
> -#define __read_mostly __attribute__((__section__(".data..read_mostly")))
> +#define __read_mostly __section(.data..read_mostly)
>
> #define INTERNODE_CACHE_SHIFT CONFIG_X86_INTERNODE_CACHE_SHIFT
> #define INTERNODE_CACHE_BYTES (1 << INTERNODE_CACHE_SHIFT)
> diff --git a/arch/x86/include/asm/intel-mid.h b/arch/x86/include/asm/intel-mid.h
> index 8e5af119dc2d..f51f04aefe1b 100644
> --- a/arch/x86/include/asm/intel-mid.h
> +++ b/arch/x86/include/asm/intel-mid.h
> @@ -43,7 +43,7 @@ struct devs_id {
>
> #define sfi_device(i) \
> static const struct devs_id *const __intel_mid_sfi_##i##_dev __used \
> - __attribute__((__section__(".x86_intel_mid_dev.init"))) = &i
> + __section(.x86_intel_mid_dev.init) = &i
>
> /**
> * struct mid_sd_board_info - template for SD device creation
> diff --git a/arch/x86/include/asm/iommu_table.h b/arch/x86/include/asm/iommu_table.h
> index 1fb3fd1a83c2..7d190710eb92 100644
> --- a/arch/x86/include/asm/iommu_table.h
> +++ b/arch/x86/include/asm/iommu_table.h
> @@ -50,9 +50,8 @@ struct iommu_table_entry {
>
> #define __IOMMU_INIT(_detect, _depend, _early_init, _late_init, _finish)\
> static const struct iommu_table_entry \
> - __iommu_entry_##_detect __used \
> - __attribute__ ((unused, __section__(".iommu_table"), \
> - aligned((sizeof(void *))))) \
> + __iommu_entry_##_detect __used __section(.iommu_table) \
> + __aligned((sizeof(void *))) \
> = {_detect, _depend, _early_init, _late_init, \
> _finish ? IOMMU_FINISH_IF_DETECTED : 0}
> /*
> diff --git a/arch/x86/include/asm/irqflags.h b/arch/x86/include/asm/irqflags.h
> index 8a0e56e1dcc9..68db90bca813 100644
> --- a/arch/x86/include/asm/irqflags.h
> +++ b/arch/x86/include/asm/irqflags.h
> @@ -9,7 +9,7 @@
> #include <asm/nospec-branch.h>
>
> /* Provide __cpuidle; we can't safely include <linux/cpu.h> */
> -#define __cpuidle __attribute__((__section__(".cpuidle.text")))
> +#define __cpuidle __section(.cpuidle.text)
>
> /*
> * Interrupt control:
> diff --git a/arch/x86/include/asm/mem_encrypt.h b/arch/x86/include/asm/mem_encrypt.h
> index 0c196c47d621..db2cd3709148 100644
> --- a/arch/x86/include/asm/mem_encrypt.h
> +++ b/arch/x86/include/asm/mem_encrypt.h
> @@ -50,7 +50,7 @@ void __init mem_encrypt_free_decrypted_mem(void);
> bool sme_active(void);
> bool sev_active(void);
>
> -#define __bss_decrypted __attribute__((__section__(".bss..decrypted")))
> +#define __bss_decrypted __section(.bss..decrypted)
>
> #else /* !CONFIG_AMD_MEM_ENCRYPT */
>
> diff --git a/arch/x86/kernel/cpu/cpu.h b/arch/x86/kernel/cpu/cpu.h
> index c0e2407abdd6..7ff9dc41a603 100644
> --- a/arch/x86/kernel/cpu/cpu.h
> +++ b/arch/x86/kernel/cpu/cpu.h
> @@ -38,8 +38,7 @@ struct _tlb_table {
>
> #define cpu_dev_register(cpu_devX) \
> static const struct cpu_dev *const __cpu_dev_##cpu_devX __used \
> - __attribute__((__section__(".x86_cpu_dev.init"))) = \
> - &cpu_devX;
> + __section(.x86_cpu_dev.init) = &cpu_devX;
>
> extern const struct cpu_dev *const __x86_cpu_dev_start[],
> *const __x86_cpu_dev_end[];
> --
> 2.23.0.rc1.153.gdeed80330f-goog
>
^ permalink raw reply
* Re: [PATCH 16/16] compiler_attributes.h: add note about __section
From: Sedat Dilek @ 2019-08-19 18:01 UTC (permalink / raw)
To: Nick Desaulniers
Cc: akpm, jpoimboe, yhs, Miguel Ojeda, Clang-Built-Linux ML,
Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
linux-kernel, netdev, bpf
In-Reply-To: <20190812215052.71840-16-ndesaulniers@google.com>
On Mon, Aug 12, 2019 at 11:53 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> The antipattern described can be found with:
> $ grep -e __section\(\" -r -e __section__\(\"
>
> Link: https://bugs.llvm.org/show_bug.cgi?id=42950
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
Tested-by: Sedat Dilek <sedat.dilek@gmail.com> [ Linux v5.3-rc5 ]
Patchset "for-5.3/x86-section-name-escaping":
include/linux/compiler.h: remove unused KENTRY macro
include/linux: prefer __section from compiler_attributes.h
include/asm-generic: prefer __section from compiler_attributes.h
x86: prefer __section from compiler_attributes.h
Thanks.
- Sedat -
> ---
> include/linux/compiler_attributes.h | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h
> index 6b318efd8a74..f8c008d7f616 100644
> --- a/include/linux/compiler_attributes.h
> +++ b/include/linux/compiler_attributes.h
> @@ -225,6 +225,16 @@
> #define __pure __attribute__((__pure__))
>
> /*
> + * Note: Since this macro makes use of the "stringification operator" `#`, a
> + * quoted string literal should not be passed to it. eg.
> + * prefer:
> + * __section(.foo)
> + * to:
> + * __section(".foo")
> + * unless the section name is dynamically built up, in which case the
> + * verbose __attribute__((__section__(".foo" x))) should be preferred.
> + * See also: https://bugs.llvm.org/show_bug.cgi?id=42950
> + *
> * gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-section-function-attribute
> * gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#index-section-variable-attribute
> * clang: https://clang.llvm.org/docs/AttributeReference.html#section-declspec-allocate
> --
> 2.23.0.rc1.153.gdeed80330f-goog
>
^ permalink raw reply
* Re: [PATCH net-next 3/6] net: dsa: enable and disable all ports
From: Vivien Didelot @ 2019-08-19 18:03 UTC (permalink / raw)
To: Marek Behun; +Cc: netdev, davem, f.fainelli, andrew
In-Reply-To: <20190819193246.0e40a1d4@nic.cz>
Hi Marek,
On Mon, 19 Aug 2019 19:32:46 +0200, Marek Behun <marek.behun@nic.cz> wrote:
> > Call the .port_enable and .port_disable functions for all ports,
> > not only the user ports, so that drivers may optimize the power
> > consumption of all ports after a successful setup.
> >
> > Unused ports are now disabled on setup. CPU and DSA ports are now
> > enabled on setup and disabled on teardown. User ports were already
> > enabled at slave creation and disabled at slave destruction.
>
> My original reason for enabling CPU and DSA ports is that enabling
> serdes irq could not be done in .setup in mv88e6xxx, since the required
> phylink structures did not yet exists for those ports.
>
> The case after this patch would be that .port_enable is called for
> CPU/DSA ports right after these required phylink structures are created
> for this port. A thought came to me while reading this that some driver
> in the future can expect, in their implementation of
> port_enable/port_disable, that phylink structures already exist for all
> ports, not just the one being currently enabled/disabled.
>
> Wouldn't it be safer if CPU/DSA ports were enabled in setup after all
> ports are registered, and disabled in teardown before ports are
> unregistered?
>
> Current:
> ->setup()
> for each port
> dsa_port_link_register_of()
> dsa_port_enable()
>
> Proposed:
> ->setup()
> for each port
> dsa_port_link_register_of()
> for each port
> dsa_port_enable()
I understand what you mean, but the scope of .port_enable really is the
port itself, so I do not expect a driver to configure something else. Also,
I prefer to keep it simple at the moment and not speculate about what a
driver may need in the future, as we may also wonder which switch must be
enabled first in a tree, etc. If that case happens in the future, we can
definitely isolate the ports enabling code after the tree is setup.
So if this series meets your requirement for now, I'd keep it like that.
Thanks,
Vivien
^ permalink raw reply
* Re: [PATCH 15/16] include/linux/compiler.h: remove unused KENTRY macro
From: Sedat Dilek @ 2019-08-19 18:03 UTC (permalink / raw)
To: Nick Desaulniers
Cc: akpm, jpoimboe, yhs, Miguel Ojeda, Clang-Built-Linux ML,
Luc Van Oostenryck, Alexei Starovoitov, Daniel Borkmann,
Martin KaFai Lau, Song Liu, linux-sparse, linux-kernel, netdev,
bpf
In-Reply-To: <20190812215052.71840-15-ndesaulniers@google.com>
On Mon, Aug 12, 2019 at 11:53 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> This macro is not used throughout the kernel. Delete it rather than
> update the __section to be a fully spelled out
> __attribute__((__section__())) to avoid
> https://bugs.llvm.org/show_bug.cgi?id=42950.
>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
Tested-by: Sedat Dilek <sedat.dilek@gmail.com> [ Linux v5.3-rc5 ]
Patchset "for-5.3/x86-section-name-escaping" (5 patches):
compiler_attributes.h: add note about __section
include/linux/compiler.h: remove unused KENTRY macro
include/linux: prefer __section from compiler_attributes.h
include/asm-generic: prefer __section from compiler_attributes.h
x86: prefer __section from compiler_attributes.h
Thanks.
- Sedat -
> ---
> include/linux/compiler.h | 23 -----------------------
> 1 file changed, 23 deletions(-)
>
> diff --git a/include/linux/compiler.h b/include/linux/compiler.h
> index 5e88e7e33abe..f01c1e527f85 100644
> --- a/include/linux/compiler.h
> +++ b/include/linux/compiler.h
> @@ -136,29 +136,6 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
> } while (0)
> #endif
>
> -/*
> - * KENTRY - kernel entry point
> - * This can be used to annotate symbols (functions or data) that are used
> - * without their linker symbol being referenced explicitly. For example,
> - * interrupt vector handlers, or functions in the kernel image that are found
> - * programatically.
> - *
> - * Not required for symbols exported with EXPORT_SYMBOL, or initcalls. Those
> - * are handled in their own way (with KEEP() in linker scripts).
> - *
> - * KENTRY can be avoided if the symbols in question are marked as KEEP() in the
> - * linker script. For example an architecture could KEEP() its entire
> - * boot/exception vector code rather than annotate each function and data.
> - */
> -#ifndef KENTRY
> -# define KENTRY(sym) \
> - extern typeof(sym) sym; \
> - static const unsigned long __kentry_##sym \
> - __used \
> - __section("___kentry" "+" #sym ) \
> - = (unsigned long)&sym;
> -#endif
> -
> #ifndef RELOC_HIDE
> # define RELOC_HIDE(ptr, off) \
> ({ unsigned long __ptr; \
> --
> 2.23.0.rc1.153.gdeed80330f-goog
>
^ permalink raw reply
* Re: [PATCH 14/16] include/linux: prefer __section from compiler_attributes.h
From: Sedat Dilek @ 2019-08-19 18:05 UTC (permalink / raw)
To: Nick Desaulniers
Cc: akpm, jpoimboe, yhs, Miguel Ojeda, Clang-Built-Linux ML,
Luc Van Oostenryck, Lai Jiangshan, Paul E. McKenney,
Josh Triplett, Steven Rostedt, Mathieu Desnoyers,
Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
Thomas Gleixner, Ingo Molnar, Peter Zijlstra (Intel),
Nicholas Piggin, Jiri Kosina, Will Deacon, Ard Biesheuvel,
Michael Ellerman, Masahiro Yamada, Hans Liljestrand,
Elena Reshetova, David Windsor, Marc Zyngier, Ming Lei,
Dou Liyang, Julien Thierry, Mauro Carvalho Chehab, Jens Axboe,
linux-kernel, linux-sparse, rcu, netdev, bpf
In-Reply-To: <20190812215052.71840-14-ndesaulniers@google.com>
On Mon, Aug 12, 2019 at 11:53 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/619
> Reported-by: Sedat Dilek <sedat.dilek@gmail.com>
> Suggested-by: Josh Poimboeuf <jpoimboe@redhat.com>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
Tested-by: Sedat Dilek <sedat.dilek@gmail.com> [ Linux v5.3-rc5 ]
Patchset "for-5.3/x86-section-name-escaping" (5 patches):
compiler_attributes.h: add note about __section
include/linux/compiler.h: remove unused KENTRY macro
include/linux: prefer __section from compiler_attributes.h
include/asm-generic: prefer __section from compiler_attributes.h
x86: prefer __section from compiler_attributes.h
Thanks.
- Sedat -
> ---
> include/linux/cache.h | 6 +++---
> include/linux/compiler.h | 8 ++++----
> include/linux/cpu.h | 2 +-
> include/linux/export.h | 2 +-
> include/linux/init_task.h | 4 ++--
> include/linux/interrupt.h | 5 ++---
> include/linux/sched/debug.h | 2 +-
> include/linux/srcutree.h | 2 +-
> 8 files changed, 15 insertions(+), 16 deletions(-)
>
> diff --git a/include/linux/cache.h b/include/linux/cache.h
> index 750621e41d1c..3f4df9eef1e1 100644
> --- a/include/linux/cache.h
> +++ b/include/linux/cache.h
> @@ -28,7 +28,7 @@
> * but may get written to during init, so can't live in .rodata (via "const").
> */
> #ifndef __ro_after_init
> -#define __ro_after_init __attribute__((__section__(".data..ro_after_init")))
> +#define __ro_after_init __section(.data..ro_after_init)
> #endif
>
> #ifndef ____cacheline_aligned
> @@ -45,8 +45,8 @@
>
> #ifndef __cacheline_aligned
> #define __cacheline_aligned \
> - __attribute__((__aligned__(SMP_CACHE_BYTES), \
> - __section__(".data..cacheline_aligned")))
> + __aligned(SMP_CACHE_BYTES) \
> + __section(.data..cacheline_aligned)
> #endif /* __cacheline_aligned */
>
> #ifndef __cacheline_aligned_in_smp
> diff --git a/include/linux/compiler.h b/include/linux/compiler.h
> index f0fd5636fddb..5e88e7e33abe 100644
> --- a/include/linux/compiler.h
> +++ b/include/linux/compiler.h
> @@ -24,7 +24,7 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
> long ______r; \
> static struct ftrace_likely_data \
> __aligned(4) \
> - __section("_ftrace_annotated_branch") \
> + __section(_ftrace_annotated_branch) \
> ______f = { \
> .data.func = __func__, \
> .data.file = __FILE__, \
> @@ -60,7 +60,7 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
> #define __trace_if_value(cond) ({ \
> static struct ftrace_branch_data \
> __aligned(4) \
> - __section("_ftrace_branch") \
> + __section(_ftrace_branch) \
> __if_trace = { \
> .func = __func__, \
> .file = __FILE__, \
> @@ -118,7 +118,7 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
> ".popsection\n\t"
>
> /* Annotate a C jump table to allow objtool to follow the code flow */
> -#define __annotate_jump_table __section(".rodata..c_jump_table")
> +#define __annotate_jump_table __section(.rodata..c_jump_table)
>
> #else
> #define annotate_reachable()
> @@ -298,7 +298,7 @@ unsigned long read_word_at_a_time(const void *addr)
> * visible to the compiler.
> */
> #define __ADDRESSABLE(sym) \
> - static void * __section(".discard.addressable") __used \
> + static void * __section(.discard.addressable) __used \
> __PASTE(__addressable_##sym, __LINE__) = (void *)&sym;
>
> /**
> diff --git a/include/linux/cpu.h b/include/linux/cpu.h
> index fcb1386bb0d4..186bbd79d6ce 100644
> --- a/include/linux/cpu.h
> +++ b/include/linux/cpu.h
> @@ -166,7 +166,7 @@ void cpu_startup_entry(enum cpuhp_state state);
> void cpu_idle_poll_ctrl(bool enable);
>
> /* Attach to any functions which should be considered cpuidle. */
> -#define __cpuidle __attribute__((__section__(".cpuidle.text")))
> +#define __cpuidle __section(.cpuidle.text)
>
> bool cpu_in_idle(unsigned long pc);
>
> diff --git a/include/linux/export.h b/include/linux/export.h
> index fd8711ed9ac4..808c1a0c2ef9 100644
> --- a/include/linux/export.h
> +++ b/include/linux/export.h
> @@ -104,7 +104,7 @@ struct kernel_symbol {
> * discarded in the final link stage.
> */
> #define __ksym_marker(sym) \
> - static int __ksym_marker_##sym[0] __section(".discard.ksym") __used
> + static int __ksym_marker_##sym[0] __section(.discard.ksym) __used
>
> #define __EXPORT_SYMBOL(sym, sec) \
> __ksym_marker(sym); \
> diff --git a/include/linux/init_task.h b/include/linux/init_task.h
> index 6049baa5b8bc..50139505da34 100644
> --- a/include/linux/init_task.h
> +++ b/include/linux/init_task.h
> @@ -51,12 +51,12 @@ extern struct cred init_cred;
>
> /* Attach to the init_task data structure for proper alignment */
> #ifdef CONFIG_ARCH_TASK_STRUCT_ON_STACK
> -#define __init_task_data __attribute__((__section__(".data..init_task")))
> +#define __init_task_data __section(.data..init_task)
> #else
> #define __init_task_data /**/
> #endif
>
> /* Attach to the thread_info data structure for proper alignment */
> -#define __init_thread_info __attribute__((__section__(".data..init_thread_info")))
> +#define __init_thread_info __section(.data..init_thread_info)
>
> #endif
> diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
> index 5b8328a99b2a..29debfe4dd0f 100644
> --- a/include/linux/interrupt.h
> +++ b/include/linux/interrupt.h
> @@ -741,8 +741,7 @@ extern int arch_early_irq_init(void);
> /*
> * We want to know which function is an entrypoint of a hardirq or a softirq.
> */
> -#define __irq_entry __attribute__((__section__(".irqentry.text")))
> -#define __softirq_entry \
> - __attribute__((__section__(".softirqentry.text")))
> +#define __irq_entry __section(.irqentry.text)
> +#define __softirq_entry __section(.softirqentry.text)
>
> #endif
> diff --git a/include/linux/sched/debug.h b/include/linux/sched/debug.h
> index 95fb9e025247..e17b66221fdd 100644
> --- a/include/linux/sched/debug.h
> +++ b/include/linux/sched/debug.h
> @@ -42,7 +42,7 @@ extern void proc_sched_set_task(struct task_struct *p);
> #endif
>
> /* Attach to any functions which should be ignored in wchan output. */
> -#define __sched __attribute__((__section__(".sched.text")))
> +#define __sched __section(.sched.text)
>
> /* Linker adds these: start and end of __sched functions */
> extern char __sched_text_start[], __sched_text_end[];
> diff --git a/include/linux/srcutree.h b/include/linux/srcutree.h
> index 9cfcc8a756ae..9de652f4e1bd 100644
> --- a/include/linux/srcutree.h
> +++ b/include/linux/srcutree.h
> @@ -124,7 +124,7 @@ struct srcu_struct {
> # define __DEFINE_SRCU(name, is_static) \
> is_static struct srcu_struct name; \
> struct srcu_struct * const __srcu_struct_##name \
> - __section("___srcu_struct_ptrs") = &name
> + __section(___srcu_struct_ptrs) = &name
> #else
> # define __DEFINE_SRCU(name, is_static) \
> static DEFINE_PER_CPU(struct srcu_data, name##_srcu_data); \
> --
> 2.23.0.rc1.153.gdeed80330f-goog
>
^ permalink raw reply
* Re: [RFC PATCH bpf-next 00/14] xdp_flow: Flow offload to XDP
From: Jakub Kicinski @ 2019-08-19 18:15 UTC (permalink / raw)
To: Toshiaki Makita
Cc: Stanislav Fomichev, Alexei Starovoitov, Daniel Borkmann,
Martin KaFai Lau, Song Liu, Yonghong Song, David S. Miller,
Jesper Dangaard Brouer, John Fastabend, Jamal Hadi Salim,
Cong Wang, Jiri Pirko, netdev, bpf, William Tu
In-Reply-To: <5e9bee13-a746-f148-00de-feb7cb7b1403@gmail.com>
On Sat, 17 Aug 2019 23:01:59 +0900, Toshiaki Makita wrote:
> On 19/08/17 (土) 3:52:24, Jakub Kicinski wrote:
> > On Fri, 16 Aug 2019 10:28:10 +0900, Toshiaki Makita wrote:
> >> On 2019/08/16 4:22, Jakub Kicinski wrote:
> >>> There's a certain allure in bringing the in-kernel BPF translation
> >>> infrastructure forward. OTOH from system architecture perspective IMHO
> >>> it does seem like a task best handed in user space. bpfilter can replace
> >>> iptables completely, here we're looking at an acceleration relatively
> >>> loosely coupled with flower.
> >>
> >> I don't think it's loosely coupled. Emulating TC behavior in userspace
> >> is not so easy.
> >>
> >> Think about recent multi-mask support in flower. Previously userspace could
> >> assume there is one mask and hash table for each preference in TC. After the
> >> change TC accepts different masks with the same pref. Such a change tends to
> >> break userspace emulation. It may ignore masks passed from flow insertion
> >> and use the mask remembered when the first flow of the pref is inserted. It
> >> may override the mask of all existing flows with the pref. It may fail to
> >> insert such flows. Any of them would result in unexpected wrong datapath
> >> handling which is critical.
> >> I think such an emulation layer needs to be updated in sync with TC.
> >
> > Oh, so you're saying that if xdp_flow is merged all patches to
> > cls_flower and netfilter which affect flow offload will be required
> > to update xdp_flow as well?
>
> Hmm... you are saying that we are allowed to break other in-kernel
> subsystem by some change? Sounds strange...
No I'm not saying that, please don't put words in my mouth.
I'm asking you if that's your intention.
Having an implementation nor support a feature of another implementation
and degrade gracefully to the slower one is not necessarily breakage.
We need to make a concious decision here, hence the clarifying question.
> > That's a question of policy. Technically the implementation in user
> > space is equivalent.
> >
> > The advantage of user space implementation is that you can add more
> > to it and explore use cases which do not fit in the flow offload API,
> > but are trivial for BPF. Not to mention the obvious advantage of
> > decoupling the upgrade path.
>
> I understand the advantage, but I can't trust such a third-party kernel
> emulation solution for this kind of thing which handles critical data path.
That's a strange argument to make. All production data path BPF today
comes from user space.
> > Personally I'm not happy with the way this patch set messes with the
> > flow infrastructure. You should use the indirect callback
> > infrastructure instead, and that way you can build the whole thing
> > touching none of the flow offload core.
>
> I don't want to mess up the core flow infrastructure either. I'm all
> ears about less invasive ways. Using indirect callback sounds like a
> good idea. Will give it a try. Many thanks.
Excellent, thanks!
^ permalink raw reply
* Re: [PATCH 00/16] treewide: prefer __section from compiler_attributes.h
From: Sedat Dilek @ 2019-08-19 18:18 UTC (permalink / raw)
To: Nick Desaulniers
Cc: akpm, jpoimboe, yhs, Miguel Ojeda, Clang-Built-Linux ML,
Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
netdev, bpf
In-Reply-To: <20190812215052.71840-17-ndesaulniers@google.com>
On Mon, Aug 12, 2019 at 11:53 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> GCC unescapes escaped string section names while Clang does not. Because
> __section uses the `#` stringification operator for the section name, it
> doesn't need to be escaped.
>
> This fixes an Oops observed in distro's that use systemd and not
> net.core.bpf_jit_enable=1, when their kernels are compiled with Clang.
>
> Instead, we should:
> 1. Prefer __section(.section_name_no_quotes).
> 2. Only use __attribute__((__section(".section"))) when creating the
> section name via C preprocessor (see the definition of __define_initcall
> in arch/um/include/shared/init.h).
>
> This antipattern was found with:
> $ grep -e __section\(\" -e __section__\(\" -r
>
> See the discussions in:
> https://bugs.llvm.org/show_bug.cgi?id=42950
> https://marc.info/?l=linux-netdev&m=156412960619946&w=2
>
> Nick Desaulniers (16):
> s390/boot: fix section name escaping
> arc: prefer __section from compiler_attributes.h
> parisc: prefer __section from compiler_attributes.h
> um: prefer __section from compiler_attributes.h
> sh: prefer __section from compiler_attributes.h
> ia64: prefer __section from compiler_attributes.h
> arm: prefer __section from compiler_attributes.h
> mips: prefer __section from compiler_attributes.h
> sparc: prefer __section from compiler_attributes.h
> powerpc: prefer __section and __printf from compiler_attributes.h
> x86: prefer __section from compiler_attributes.h
> arm64: prefer __section from compiler_attributes.h
> include/asm-generic: prefer __section from compiler_attributes.h
> include/linux: prefer __section from compiler_attributes.h
> include/linux/compiler.h: remove unused KENTRY macro
> compiler_attributes.h: add note about __section
>
Hi Nick,
thanks for this patchset and the nice section names cleanup and simplification.
I have tested 5 relevant patches for my x86-64 Debian/buster system.
Patchset "for-5.3/x86-section-name-escaping" (5 patches):
compiler_attributes.h: add note about __section
include/linux/compiler.h: remove unused KENTRY macro
include/linux: prefer __section from compiler_attributes.h
include/asm-generic: prefer __section from compiler_attributes.h
x86: prefer __section from compiler_attributes.h
Toolchain: LLVM/Clang compiler and LLD linker version 9.0.0-rc2 (from
Debian/experimental)
I can boot on bare metal.
$ cat /proc/version
Linux version 5.3.0-rc5-1-amd64-cbl-asmgoto
(sedat.dilek@gmail.com@iniza) (clang version 9.0.0-+rc2-1~exp1
(tags/RELEASE_900/rc2)) #1~buster+dileks1 SMP 2019-08-19
I have sent by Tested-by to the single patches.
Have a nice day,
- Sedat -
> arch/arc/include/asm/linkage.h | 8 +++----
> arch/arc/include/asm/mach_desc.h | 3 +--
> arch/arm/include/asm/cache.h | 2 +-
> arch/arm/include/asm/mach/arch.h | 4 ++--
> arch/arm/include/asm/setup.h | 2 +-
> arch/arm64/include/asm/cache.h | 2 +-
> arch/arm64/kernel/smp_spin_table.c | 2 +-
> arch/ia64/include/asm/cache.h | 2 +-
> arch/mips/include/asm/cache.h | 2 +-
> arch/parisc/include/asm/cache.h | 2 +-
> arch/parisc/include/asm/ldcw.h | 2 +-
> arch/powerpc/boot/main.c | 3 +--
> arch/powerpc/boot/ps3.c | 6 ++----
> arch/powerpc/include/asm/cache.h | 2 +-
> arch/powerpc/kernel/btext.c | 2 +-
> arch/s390/boot/startup.c | 2 +-
> arch/sh/include/asm/cache.h | 2 +-
> arch/sparc/include/asm/cache.h | 2 +-
> arch/sparc/kernel/btext.c | 2 +-
> arch/um/kernel/um_arch.c | 6 +++---
> arch/x86/include/asm/cache.h | 2 +-
> arch/x86/include/asm/intel-mid.h | 2 +-
> arch/x86/include/asm/iommu_table.h | 5 ++---
> arch/x86/include/asm/irqflags.h | 2 +-
> arch/x86/include/asm/mem_encrypt.h | 2 +-
> arch/x86/kernel/cpu/cpu.h | 3 +--
> include/asm-generic/error-injection.h | 2 +-
> include/asm-generic/kprobes.h | 5 ++---
> include/linux/cache.h | 6 +++---
> include/linux/compiler.h | 31 ++++-----------------------
> include/linux/compiler_attributes.h | 10 +++++++++
> include/linux/cpu.h | 2 +-
> include/linux/export.h | 2 +-
> include/linux/init_task.h | 4 ++--
> include/linux/interrupt.h | 5 ++---
> include/linux/sched/debug.h | 2 +-
> include/linux/srcutree.h | 2 +-
> 37 files changed, 62 insertions(+), 83 deletions(-)
>
> --
> 2.23.0.rc1.153.gdeed80330f-goog
>
^ permalink raw reply
* [net PATCH] net/smc: make sure EPOLLOUT is raised
From: Jason Baron @ 2019-08-19 18:36 UTC (permalink / raw)
To: davem; +Cc: netdev, Eric Dumazet, Ursula Braun, Karsten Graul
Currently, we are only explicitly setting SOCK_NOSPACE on a write timeout
for non-blocking sockets. Epoll() edge-trigger mode relies on SOCK_NOSPACE
being set when -EAGAIN is returned to ensure that EPOLLOUT is raised.
Expand the setting of SOCK_NOSPACE to non-blocking sockets as well that can
use SO_SNDTIMEO to adjust their write timeout. This mirrors the behavior
that Eric Dumazet introduced for tcp sockets.
Signed-off-by: Jason Baron <jbaron@akamai.com>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Ursula Braun <ubraun@linux.ibm.com>
Cc: Karsten Graul <kgraul@linux.ibm.com>
---
net/smc/smc_tx.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/net/smc/smc_tx.c b/net/smc/smc_tx.c
index f0de323..6c8f09c 100644
--- a/net/smc/smc_tx.c
+++ b/net/smc/smc_tx.c
@@ -76,13 +76,11 @@ static int smc_tx_wait(struct smc_sock *smc, int flags)
DEFINE_WAIT_FUNC(wait, woken_wake_function);
struct smc_connection *conn = &smc->conn;
struct sock *sk = &smc->sk;
- bool noblock;
long timeo;
int rc = 0;
/* similar to sk_stream_wait_memory */
timeo = sock_sndtimeo(sk, flags & MSG_DONTWAIT);
- noblock = timeo ? false : true;
add_wait_queue(sk_sleep(sk), &wait);
while (1) {
sk_set_bit(SOCKWQ_ASYNC_NOSPACE, sk);
@@ -97,8 +95,8 @@ static int smc_tx_wait(struct smc_sock *smc, int flags)
break;
}
if (!timeo) {
- if (noblock)
- set_bit(SOCK_NOSPACE, &sk->sk_socket->flags);
+ /* ensure EPOLLOUT is subsequently generated */
+ set_bit(SOCK_NOSPACE, &sk->sk_socket->flags);
rc = -EAGAIN;
break;
}
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net] tcp: make sure EPOLLOUT wont be missed
From: Jason Baron @ 2019-08-19 18:40 UTC (permalink / raw)
To: Eric Dumazet, Eric Dumazet, David S . Miller
Cc: netdev, Soheil Hassas Yeganeh, Neal Cardwell, Vladimir Rutsky
In-Reply-To: <c31d7c04-9d6f-aefb-500e-5b9b635ff221@gmail.com>
On 8/17/19 12:26 PM, Eric Dumazet wrote:
>
>
> On 8/17/19 4:19 PM, Jason Baron wrote:
>>
>>
>> On 8/17/19 12:26 AM, Eric Dumazet wrote:
>>> As Jason Baron explained in commit 790ba4566c1a ("tcp: set SOCK_NOSPACE
>>> under memory pressure"), it is crucial we properly set SOCK_NOSPACE
>>> when needed.
>>>
>>> However, Jason patch had a bug, because the 'nonblocking' status
>>> as far as sk_stream_wait_memory() is concerned is governed
>>> by MSG_DONTWAIT flag passed at sendmsg() time :
>>>
>>> long timeo = sock_sndtimeo(sk, flags & MSG_DONTWAIT);
>>>
>>> So it is very possible that tcp sendmsg() calls sk_stream_wait_memory(),
>>> and that sk_stream_wait_memory() returns -EAGAIN with SOCK_NOSPACE
>>> cleared, if sk->sk_sndtimeo has been set to a small (but not zero)
>>> value.
>>
>> Is MSG_DONTWAIT not set in this case? The original patch was intended
>> only for the explicit non-blocking case. The epoll manpage says:
>> "EPOLLET flag should use nonblocking file descriptors". So the original
>> intention was not to impact the blocking case. This seems to me like
>> a different use-case.
>>
>
> I guess the problem is how we define 'non-blocking' ...
>
> SO_SNDTIMEO can be used by application to implement a variation of non-blocking,
> by waiting for a socket event with a short timeout, to maybe recover
> from memory pressure conditions in a more efficient way than simply looping.
>
> Note that the man page for epoll() only _suggests_ to use nonblocking file descriptors.
>
> <quote>
> The suggested way to use epoll as an edge-triggered (EPOLLET)
> interface is as follows:
>
> i with nonblocking file descriptors; and
>
> ii by waiting for an event only after read(2) or
> write(2) return EAGAIN.
> </quote>
>
>
Ok, seems reasonable:
Acked-by: Jason Baron <jbaron@akamai.com>
I found a similar pattern in net/smc/smc_tx.c, which I also just sent a
patch for.
Thanks,
-Jason
^ permalink raw reply
* [PATCH 4/5] netfilter: xt_nfacct: Fix alignment mismatch in xt_nfacct_match_info
From: Pablo Neira Ayuso @ 2019-08-19 18:49 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20190819184911.15263-1-pablo@netfilter.org>
From: Juliana Rodrigueiro <juliana.rodrigueiro@intra2net.com>
When running a 64-bit kernel with a 32-bit iptables binary, the size of
the xt_nfacct_match_info struct diverges.
kernel: sizeof(struct xt_nfacct_match_info) : 40
iptables: sizeof(struct xt_nfacct_match_info)) : 36
Trying to append nfacct related rules results in an unhelpful message.
Although it is suggested to look for more information in dmesg, nothing
can be found there.
# iptables -A <chain> -m nfacct --nfacct-name <acct-object>
iptables: Invalid argument. Run `dmesg' for more information.
This patch fixes the memory misalignment by enforcing 8-byte alignment
within the struct's first revision. This solution is often used in many
other uapi netfilter headers.
Signed-off-by: Juliana Rodrigueiro <juliana.rodrigueiro@intra2net.com>
Acked-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
include/uapi/linux/netfilter/xt_nfacct.h | 5 +++++
net/netfilter/xt_nfacct.c | 36 ++++++++++++++++++++++----------
2 files changed, 30 insertions(+), 11 deletions(-)
diff --git a/include/uapi/linux/netfilter/xt_nfacct.h b/include/uapi/linux/netfilter/xt_nfacct.h
index 5c8a4d760ee3..b5123ab8d54a 100644
--- a/include/uapi/linux/netfilter/xt_nfacct.h
+++ b/include/uapi/linux/netfilter/xt_nfacct.h
@@ -11,4 +11,9 @@ struct xt_nfacct_match_info {
struct nf_acct *nfacct;
};
+struct xt_nfacct_match_info_v1 {
+ char name[NFACCT_NAME_MAX];
+ struct nf_acct *nfacct __attribute__((aligned(8)));
+};
+
#endif /* _XT_NFACCT_MATCH_H */
diff --git a/net/netfilter/xt_nfacct.c b/net/netfilter/xt_nfacct.c
index d0ab1adf5bff..5aab6df74e0f 100644
--- a/net/netfilter/xt_nfacct.c
+++ b/net/netfilter/xt_nfacct.c
@@ -54,25 +54,39 @@ nfacct_mt_destroy(const struct xt_mtdtor_param *par)
nfnl_acct_put(info->nfacct);
}
-static struct xt_match nfacct_mt_reg __read_mostly = {
- .name = "nfacct",
- .family = NFPROTO_UNSPEC,
- .checkentry = nfacct_mt_checkentry,
- .match = nfacct_mt,
- .destroy = nfacct_mt_destroy,
- .matchsize = sizeof(struct xt_nfacct_match_info),
- .usersize = offsetof(struct xt_nfacct_match_info, nfacct),
- .me = THIS_MODULE,
+static struct xt_match nfacct_mt_reg[] __read_mostly = {
+ {
+ .name = "nfacct",
+ .revision = 0,
+ .family = NFPROTO_UNSPEC,
+ .checkentry = nfacct_mt_checkentry,
+ .match = nfacct_mt,
+ .destroy = nfacct_mt_destroy,
+ .matchsize = sizeof(struct xt_nfacct_match_info),
+ .usersize = offsetof(struct xt_nfacct_match_info, nfacct),
+ .me = THIS_MODULE,
+ },
+ {
+ .name = "nfacct",
+ .revision = 1,
+ .family = NFPROTO_UNSPEC,
+ .checkentry = nfacct_mt_checkentry,
+ .match = nfacct_mt,
+ .destroy = nfacct_mt_destroy,
+ .matchsize = sizeof(struct xt_nfacct_match_info_v1),
+ .usersize = offsetof(struct xt_nfacct_match_info_v1, nfacct),
+ .me = THIS_MODULE,
+ },
};
static int __init nfacct_mt_init(void)
{
- return xt_register_match(&nfacct_mt_reg);
+ return xt_register_matches(nfacct_mt_reg, ARRAY_SIZE(nfacct_mt_reg));
}
static void __exit nfacct_mt_exit(void)
{
- xt_unregister_match(&nfacct_mt_reg);
+ xt_unregister_matches(nfacct_mt_reg, ARRAY_SIZE(nfacct_mt_reg));
}
module_init(nfacct_mt_init);
--
2.11.0
^ permalink raw reply related
* [PATCH 1/5] MAINTAINERS: Remove IP MASQUERADING record
From: Pablo Neira Ayuso @ 2019-08-19 18:49 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20190819184911.15263-1-pablo@netfilter.org>
From: Denis Efremov <efremov@linux.com>
This entry is in MAINTAINERS for historical purpose.
It doesn't match current sources since the commit
adf82accc5f5 ("netfilter: x_tables: merge ip and
ipv6 masquerade modules") moved the module.
The net/netfilter/xt_MASQUERADE.c module is already under
the netfilter section. Thus, there is no purpose to keep this
separate entry in MAINTAINERS.
Cc: Florian Westphal <fw@strlen.de>
Cc: Juanjo Ciarlante <jjciarla@raiz.uncu.edu.ar>
Cc: netfilter-devel@vger.kernel.org
Suggested-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Denis Efremov <efremov@linux.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
MAINTAINERS | 5 -----
1 file changed, 5 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index a416574780d6..6839cfd91dde 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8439,11 +8439,6 @@ S: Maintained
F: fs/io_uring.c
F: include/uapi/linux/io_uring.h
-IP MASQUERADING
-M: Juanjo Ciarlante <jjciarla@raiz.uncu.edu.ar>
-S: Maintained
-F: net/ipv4/netfilter/ipt_MASQUERADE.c
-
IPMI SUBSYSTEM
M: Corey Minyard <minyard@acm.org>
L: openipmi-developer@lists.sourceforge.net (moderated for non-subscribers)
--
2.11.0
^ permalink raw reply related
* [PATCH 0/5] Netfilter fixes for net
From: Pablo Neira Ayuso @ 2019-08-19 18:49 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
Hi,
The following patchset contains Netfilter fixes for net:
1) Remove IP MASQUERADING record in MAINTAINERS file,
from Denis Efremov.
2) Counter arguments are swapped in ebtables, from
Todd Seidelmann.
3) Missing netlink attribute validation in flow_offload
extension.
4) Incorrect alignment in xt_nfacct that breaks 32-bits
userspace / 64-bits kernels, from Juliana Rodrigueiro.
5) Missing include guard in nf_conntrack_h323_types.h,
from Masahiro Yamada.
You can pull these changes from:
git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git
Thanks.
----------------------------------------------------------------
The following changes since commit cfef46d692efd852a0da6803f920cc756eea2855:
ravb: Fix use-after-free ravb_tstamp_skb (2019-08-18 14:19:14 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git HEAD
for you to fetch changes up to 38a429c898ddd210cc35463b096389f97c3c5a73:
netfilter: add include guard to nf_conntrack_h323_types.h (2019-08-19 13:59:57 +0200)
----------------------------------------------------------------
Denis Efremov (1):
MAINTAINERS: Remove IP MASQUERADING record
Juliana Rodrigueiro (1):
netfilter: xt_nfacct: Fix alignment mismatch in xt_nfacct_match_info
Masahiro Yamada (1):
netfilter: add include guard to nf_conntrack_h323_types.h
Pablo Neira Ayuso (1):
netfilter: nft_flow_offload: missing netlink attribute policy
Todd Seidelmann (1):
netfilter: ebtables: Fix argument order to ADD_COUNTER
MAINTAINERS | 5 ----
include/linux/netfilter/nf_conntrack_h323_types.h | 5 ++++
include/uapi/linux/netfilter/xt_nfacct.h | 5 ++++
net/bridge/netfilter/ebtables.c | 8 ++---
net/netfilter/nft_flow_offload.c | 6 ++++
net/netfilter/xt_nfacct.c | 36 ++++++++++++++++-------
6 files changed, 45 insertions(+), 20 deletions(-)
^ permalink raw reply
* [PATCH 5/5] netfilter: add include guard to nf_conntrack_h323_types.h
From: Pablo Neira Ayuso @ 2019-08-19 18:49 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20190819184911.15263-1-pablo@netfilter.org>
From: Masahiro Yamada <yamada.masahiro@socionext.com>
Add a header include guard just in case.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
include/linux/netfilter/nf_conntrack_h323_types.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/include/linux/netfilter/nf_conntrack_h323_types.h b/include/linux/netfilter/nf_conntrack_h323_types.h
index 7a6871ac8784..74c6f9241944 100644
--- a/include/linux/netfilter/nf_conntrack_h323_types.h
+++ b/include/linux/netfilter/nf_conntrack_h323_types.h
@@ -4,6 +4,9 @@
* Copyright (c) 2006 Jing Min Zhao <zhaojingmin@users.sourceforge.net>
*/
+#ifndef _NF_CONNTRACK_H323_TYPES_H
+#define _NF_CONNTRACK_H323_TYPES_H
+
typedef struct TransportAddress_ipAddress { /* SEQUENCE */
int options; /* No use */
unsigned int ip;
@@ -931,3 +934,5 @@ typedef struct RasMessage { /* CHOICE */
InfoRequestResponse infoRequestResponse;
};
} RasMessage;
+
+#endif /* _NF_CONNTRACK_H323_TYPES_H */
--
2.11.0
^ permalink raw reply related
* [PATCH 2/5] netfilter: ebtables: Fix argument order to ADD_COUNTER
From: Pablo Neira Ayuso @ 2019-08-19 18:49 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20190819184911.15263-1-pablo@netfilter.org>
From: Todd Seidelmann <tseidelmann@linode.com>
The ordering of arguments to the x_tables ADD_COUNTER macro
appears to be wrong in ebtables (cf. ip_tables.c, ip6_tables.c,
and arp_tables.c).
This causes data corruption in the ebtables userspace tools
because they get incorrect packet & byte counts from the kernel.
Fixes: d72133e628803 ("netfilter: ebtables: use ADD_COUNTER macro")
Signed-off-by: Todd Seidelmann <tseidelmann@linode.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/bridge/netfilter/ebtables.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c
index c8177a89f52c..4096d8a74a2b 100644
--- a/net/bridge/netfilter/ebtables.c
+++ b/net/bridge/netfilter/ebtables.c
@@ -221,7 +221,7 @@ unsigned int ebt_do_table(struct sk_buff *skb,
return NF_DROP;
}
- ADD_COUNTER(*(counter_base + i), 1, skb->len);
+ ADD_COUNTER(*(counter_base + i), skb->len, 1);
/* these should only watch: not modify, nor tell us
* what to do with the packet
@@ -959,8 +959,8 @@ static void get_counters(const struct ebt_counter *oldcounters,
continue;
counter_base = COUNTER_BASE(oldcounters, nentries, cpu);
for (i = 0; i < nentries; i++)
- ADD_COUNTER(counters[i], counter_base[i].pcnt,
- counter_base[i].bcnt);
+ ADD_COUNTER(counters[i], counter_base[i].bcnt,
+ counter_base[i].pcnt);
}
}
@@ -1280,7 +1280,7 @@ static int do_update_counters(struct net *net, const char *name,
/* we add to the counters of the first cpu */
for (i = 0; i < num_counters; i++)
- ADD_COUNTER(t->private->counters[i], tmp[i].pcnt, tmp[i].bcnt);
+ ADD_COUNTER(t->private->counters[i], tmp[i].bcnt, tmp[i].pcnt);
write_unlock_bh(&t->lock);
ret = 0;
--
2.11.0
^ permalink raw reply related
* [PATCH 3/5] netfilter: nft_flow_offload: missing netlink attribute policy
From: Pablo Neira Ayuso @ 2019-08-19 18:49 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20190819184911.15263-1-pablo@netfilter.org>
The netlink attribute policy for NFTA_FLOW_TABLE_NAME is missing.
Fixes: a3c90f7a2323 ("netfilter: nf_tables: flow offload expression")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/netfilter/nft_flow_offload.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/net/netfilter/nft_flow_offload.c b/net/netfilter/nft_flow_offload.c
index 060a4ed46d5e..01705ad74a9a 100644
--- a/net/netfilter/nft_flow_offload.c
+++ b/net/netfilter/nft_flow_offload.c
@@ -149,6 +149,11 @@ static int nft_flow_offload_validate(const struct nft_ctx *ctx,
return nft_chain_validate_hooks(ctx->chain, hook_mask);
}
+static const struct nla_policy nft_flow_offload_policy[NFTA_FLOW_MAX + 1] = {
+ [NFTA_FLOW_TABLE_NAME] = { .type = NLA_STRING,
+ .len = NFT_NAME_MAXLEN - 1 },
+};
+
static int nft_flow_offload_init(const struct nft_ctx *ctx,
const struct nft_expr *expr,
const struct nlattr * const tb[])
@@ -207,6 +212,7 @@ static const struct nft_expr_ops nft_flow_offload_ops = {
static struct nft_expr_type nft_flow_offload_type __read_mostly = {
.name = "flow_offload",
.ops = &nft_flow_offload_ops,
+ .policy = nft_flow_offload_policy,
.maxattr = NFTA_FLOW_MAX,
.owner = THIS_MODULE,
};
--
2.11.0
^ permalink raw reply related
* Re: [PATCH net-next 0/1] Add BASE-T1 PHY support
From: Heiner Kallweit @ 2019-08-19 19:07 UTC (permalink / raw)
To: Christian Herber, davem@davemloft.net
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <AM6PR0402MB379864B810F08D3698618B5F86A80@AM6PR0402MB3798.eurprd04.prod.outlook.com>
On 19.08.2019 08:32, Christian Herber wrote:
> On 16.08.2019 22:59, Heiner Kallweit wrote:
>> On 15.08.2019 17:32, Christian Herber wrote:
>>> This patch adds basic support for BASE-T1 PHYs in the framework.
>>> BASE-T1 PHYs main area of application are automotive and industrial.
>>> BASE-T1 is standardized in IEEE 802.3, namely
>>> - IEEE 802.3bw: 100BASE-T1
>>> - IEEE 802.3bp 1000BASE-T1
>>> - IEEE 802.3cg: 10BASE-T1L and 10BASE-T1S
>>>
>>> There are no products which contain BASE-T1 and consumer type PHYs like
>>> 1000BASE-T. However, devices exist which combine 100BASE-T1 and 1000BASE-T1
>>> PHYs with auto-negotiation.
>>
>> Is this meant in a way that *currently* there are no PHY's combining Base-T1
>> with normal Base-T modes? Or are there reasons why this isn't possible in
>> general? I'm asking because we have PHY's combining copper and fiber, and e.g.
>> the mentioned Aquantia PHY that combines NBase-T with 1000Base-T2.
>>
>>>
>>> The intention of this patch is to make use of the existing Clause 45 functions.
>>> BASE-T1 adds some additional registers e.g. for aneg control, which follow a
>>> similiar register layout as the existing devices. The bits which are used in
>>> BASE-T1 specific registers are the same as in basic registers, thus the
>>> existing functions can be resued, with get_aneg_ctrl() selecting the correct
>>> register address.
>>>
>> If Base-T1 can't be combined with other modes then at a first glance I see no
>> benefit in defining new registers e.g. for aneg control, and the standard ones
>> are unused. Why not using the standard registers? Can you shed some light on that?
>>
>> Are the new registers internally shadowed to the standard location?
>> That's something I've seen on other PHY's: one register appears in different
>> places in different devices.
>>
>>> The current version of ethtool has been prepared for 100/1000BASE-T1 and works
>>> with this patch. 10BASE-T1 needs to be added to ethtool.
>>>
>>> Christian Herber (1):
>>> Added BASE-T1 PHY support to PHY Subsystem
>>>
>>> drivers/net/phy/phy-c45.c | 113 +++++++++++++++++++++++++++++++----
>>> drivers/net/phy/phy-core.c | 4 +-
>>> include/uapi/linux/ethtool.h | 2 +
>>> include/uapi/linux/mdio.h | 21 +++++++
>>> 4 files changed, 129 insertions(+), 11 deletions(-)
>>>
>>
>> Heiner
>>
>
> Hi Heiner,
>
> I do not think the Aquantia part you are describing is publicly
> documented, so i cannot comment on that part.
Right, datasheet isn't publicly available. All I wanted to say with
mentioning this PHY: It's not a rare exception that a PHY combines
standard BaseT modes with "non-consumer" modes for special purposes.
One practical use case of this proprietary 1000Base-T2 mode is
re-using existing 2-pair cabling in aircrafts.
> There are multiple reasons why e.g. xBASE-T1 plus 1000BASE-T is
> unlikely. First, the is no use-case known to me, where this would be
> required. Second, there is no way that you can do an auto-negotiation
> between the two, as these both have their own auto-neg defined (Clause
> 28/73 vs. Clause 98). Thirdly, if you would ever have a product with
> both, I believe it would just include two full PHYs and a way to select
> which flavor you want. Of course, this is the theory until proven
> otherwise, but to me it is sufficient to use a single driver.
>
I'm with you if you say it's unlikely. However your statement in the
commit message leaves the impression that there can't be such a device.
And that's a difference.
Regarding "including two full PHYs":
This case we have already, there are PHYs combining different IP blocks,
each one supporting a specific mode (e.g. copper and fiber). There you
also have the case of different autoneg methods, clause 28 vs. clause 37.
> The registers are different in the fields they include. It is just that
> the flags which are used by the Linux driver, like restarting auto-neg,
> are at the same position.
>
Good to know. Your commit description doesn't mention any specific PHY.
I suppose you have PHYs you'd like to operate with the genphy_c45 driver.
Could you give an example? And ideally, is a public datasheet available?
> Christian
>
>
Heiner
^ permalink raw reply
* [PATCH bpf-next v2 0/4] selftests/bpf: test_progs: misc fixes
From: Stanislav Fomichev @ 2019-08-19 19:17 UTC (permalink / raw)
To: netdev, bpf; +Cc: davem, ast, daniel, Stanislav Fomichev, Andrii Nakryiko
* add test__skip to indicate skipped tests
* remove global success/error counts (use environment)
* remove asserts from the tests
* remove unused ret from send_signal test
v2:
* drop patch that changes output to keep consistent with test_verifier
(Alexei Starovoitov)
* QCHECK instead of test__fail (Andrii Nakryiko)
* test__skip count number of subtests (Andrii Nakryiko)
Cc: Andrii Nakryiko <andriin@fb.com>
Stanislav Fomichev (4):
selftests/bpf: test_progs: test__skip
selftests/bpf: test_progs: remove global fail/success counts
selftests/bpf: test_progs: remove asserts from subtests
selftests/bpf: test_progs: remove unused ret
.../selftests/bpf/prog_tests/bpf_obj_id.c | 20 +++++----
.../bpf/prog_tests/bpf_verif_scale.c | 9 +---
.../selftests/bpf/prog_tests/flow_dissector.c | 4 +-
.../bpf/prog_tests/get_stack_raw_tp.c | 3 --
.../selftests/bpf/prog_tests/global_data.c | 20 +++------
.../selftests/bpf/prog_tests/l4lb_all.c | 8 +---
.../selftests/bpf/prog_tests/map_lock.c | 37 ++++++++--------
.../selftests/bpf/prog_tests/pkt_access.c | 4 +-
.../selftests/bpf/prog_tests/pkt_md_access.c | 4 +-
.../bpf/prog_tests/queue_stack_map.c | 8 +---
.../bpf/prog_tests/reference_tracking.c | 4 +-
.../selftests/bpf/prog_tests/send_signal.c | 43 +++++++++----------
.../selftests/bpf/prog_tests/spinlock.c | 16 +++----
.../bpf/prog_tests/stacktrace_build_id.c | 7 +--
.../bpf/prog_tests/stacktrace_build_id_nmi.c | 7 +--
.../selftests/bpf/prog_tests/stacktrace_map.c | 17 +++-----
.../bpf/prog_tests/stacktrace_map_raw_tp.c | 9 ++--
.../bpf/prog_tests/task_fd_query_rawtp.c | 3 --
.../bpf/prog_tests/task_fd_query_tp.c | 5 ---
.../selftests/bpf/prog_tests/tcp_estats.c | 4 +-
tools/testing/selftests/bpf/prog_tests/xdp.c | 4 +-
.../bpf/prog_tests/xdp_adjust_tail.c | 4 +-
.../selftests/bpf/prog_tests/xdp_noinline.c | 7 +--
tools/testing/selftests/bpf/test_progs.c | 41 ++++++++++++------
tools/testing/selftests/bpf/test_progs.h | 19 +++++---
25 files changed, 135 insertions(+), 172 deletions(-)
--
2.23.0.rc1.153.gdeed80330f-goog
^ permalink raw reply
* [PATCH bpf-next v2 1/4] selftests/bpf: test_progs: test__skip
From: Stanislav Fomichev @ 2019-08-19 19:17 UTC (permalink / raw)
To: netdev, bpf; +Cc: davem, ast, daniel, Stanislav Fomichev, Andrii Nakryiko
In-Reply-To: <20190819191752.241637-1-sdf@google.com>
Export test__skip() to indicate skipped tests and use it in
test_send_signal_nmi().
Cc: Andrii Nakryiko <andriin@fb.com>
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
.../selftests/bpf/prog_tests/send_signal.c | 1 +
tools/testing/selftests/bpf/test_progs.c | 20 +++++++++++++++++--
tools/testing/selftests/bpf/test_progs.h | 2 ++
3 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/send_signal.c b/tools/testing/selftests/bpf/prog_tests/send_signal.c
index 1575f0a1f586..40c2c5efdd3e 100644
--- a/tools/testing/selftests/bpf/prog_tests/send_signal.c
+++ b/tools/testing/selftests/bpf/prog_tests/send_signal.c
@@ -204,6 +204,7 @@ static int test_send_signal_nmi(void)
if (errno == ENOENT) {
printf("%s:SKIP:no PERF_COUNT_HW_CPU_CYCLES\n",
__func__);
+ test__skip();
return 0;
}
/* Let the test fail with a more informative message */
diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index 12895d03d58b..e545dfb55872 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -17,6 +17,7 @@ struct prog_test_def {
bool force_log;
int pass_cnt;
int error_cnt;
+ int skip_cnt;
bool tested;
const char *subtest_name;
@@ -56,6 +57,14 @@ static void dump_test_log(const struct prog_test_def *test, bool failed)
fseeko(stdout, 0, SEEK_SET); /* rewind */
}
+static void skip_account(void)
+{
+ if (env.test->skip_cnt) {
+ env.skip_cnt++;
+ env.test->skip_cnt = 0;
+ }
+}
+
void test__end_subtest()
{
struct prog_test_def *test = env.test;
@@ -65,6 +74,7 @@ void test__end_subtest()
env.fail_cnt++;
else
env.sub_succ_cnt++;
+ skip_account();
dump_test_log(test, sub_error_cnt);
@@ -105,6 +115,11 @@ void test__force_log() {
env.test->force_log = true;
}
+void test__skip(void)
+{
+ env.test->skip_cnt++;
+}
+
struct ipv4_packet pkt_v4 = {
.eth.h_proto = __bpf_constant_htons(ETH_P_IP),
.iph.ihl = 5,
@@ -510,6 +525,7 @@ int main(int argc, char **argv)
env.fail_cnt++;
else
env.succ_cnt++;
+ skip_account();
dump_test_log(test, test->error_cnt);
@@ -518,8 +534,8 @@ int main(int argc, char **argv)
test->error_cnt ? "FAIL" : "OK");
}
stdio_restore();
- printf("Summary: %d/%d PASSED, %d FAILED\n",
- env.succ_cnt, env.sub_succ_cnt, env.fail_cnt);
+ printf("Summary: %d/%d PASSED, %d SKIPPED, %d FAILED\n",
+ env.succ_cnt, env.sub_succ_cnt, env.skip_cnt, env.fail_cnt);
free(env.test_selector.num_set);
free(env.subtest_selector.num_set);
diff --git a/tools/testing/selftests/bpf/test_progs.h b/tools/testing/selftests/bpf/test_progs.h
index 37d427f5a1e5..9defd35cb6c0 100644
--- a/tools/testing/selftests/bpf/test_progs.h
+++ b/tools/testing/selftests/bpf/test_progs.h
@@ -64,6 +64,7 @@ struct test_env {
int succ_cnt; /* successful tests */
int sub_succ_cnt; /* successful sub-tests */
int fail_cnt; /* total failed tests + sub-tests */
+ int skip_cnt; /* skipped tests */
};
extern int error_cnt;
@@ -72,6 +73,7 @@ extern struct test_env env;
extern void test__force_log();
extern bool test__start_subtest(const char *name);
+extern void test__skip(void);
#define MAGIC_BYTES 123
--
2.23.0.rc1.153.gdeed80330f-goog
^ permalink raw reply related
* [PATCH bpf-next v2 2/4] selftests/bpf: test_progs: remove global fail/success counts
From: Stanislav Fomichev @ 2019-08-19 19:17 UTC (permalink / raw)
To: netdev, bpf; +Cc: davem, ast, daniel, Stanislav Fomichev, Andrii Nakryiko
In-Reply-To: <20190819191752.241637-1-sdf@google.com>
Now that we have a global per-test/per-environment state, there
is no longer need to have global fail/success counters (and there
is no need to save/get the diff before/after the test).
Introduce QCHECK macro (suggested by Andrii) and covert existing tests
to it. QCHECK uses new test__fail() to record the failure.
Cc: Andrii Nakryiko <andriin@fb.com>
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
.../selftests/bpf/prog_tests/bpf_obj_id.c | 3 +--
.../bpf/prog_tests/bpf_verif_scale.c | 9 +-------
.../selftests/bpf/prog_tests/flow_dissector.c | 4 +---
.../bpf/prog_tests/get_stack_raw_tp.c | 3 ---
.../selftests/bpf/prog_tests/global_data.c | 20 +++++-------------
.../selftests/bpf/prog_tests/l4lb_all.c | 8 ++-----
.../selftests/bpf/prog_tests/map_lock.c | 17 ++++++---------
.../selftests/bpf/prog_tests/pkt_access.c | 4 +---
.../selftests/bpf/prog_tests/pkt_md_access.c | 4 +---
.../bpf/prog_tests/queue_stack_map.c | 8 ++-----
.../bpf/prog_tests/reference_tracking.c | 4 +---
.../selftests/bpf/prog_tests/spinlock.c | 6 ++----
.../selftests/bpf/prog_tests/stacktrace_map.c | 17 +++++++--------
.../bpf/prog_tests/stacktrace_map_raw_tp.c | 9 +++-----
.../bpf/prog_tests/task_fd_query_rawtp.c | 3 ---
.../bpf/prog_tests/task_fd_query_tp.c | 5 -----
.../selftests/bpf/prog_tests/tcp_estats.c | 4 +---
tools/testing/selftests/bpf/prog_tests/xdp.c | 4 +---
.../bpf/prog_tests/xdp_adjust_tail.c | 4 +---
.../selftests/bpf/prog_tests/xdp_noinline.c | 7 ++-----
tools/testing/selftests/bpf/test_progs.c | 21 ++++++++-----------
tools/testing/selftests/bpf/test_progs.h | 17 +++++++++------
22 files changed, 58 insertions(+), 123 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_obj_id.c b/tools/testing/selftests/bpf/prog_tests/bpf_obj_id.c
index fb5840a62548..b9d0cd312839 100644
--- a/tools/testing/selftests/bpf/prog_tests/bpf_obj_id.c
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_obj_id.c
@@ -48,8 +48,7 @@ void test_bpf_obj_id(void)
/* test_obj_id.o is a dumb prog. It should never fail
* to load.
*/
- if (err)
- error_cnt++;
+ QCHECK(err);
assert(!err);
/* Insert a magic value to the map */
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 1a1eae356f81..d0fd20d021a7 100644
--- a/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c
@@ -28,8 +28,6 @@ static int check_load(const char *file, enum bpf_prog_type type)
attr.prog_flags = BPF_F_TEST_RND_HI32;
err = bpf_prog_load_xattr(&attr, &obj, &prog_fd);
bpf_object__close(obj);
- if (err)
- error_cnt++;
return err;
}
@@ -105,12 +103,7 @@ void test_bpf_verif_scale(void)
continue;
err = check_load(test->file, test->attach_type);
- if (test->fails) { /* expected to fail */
- if (err)
- error_cnt--;
- else
- error_cnt++;
- }
+ QCHECK(err && !test->fails);
}
if (env.verifier_stats)
diff --git a/tools/testing/selftests/bpf/prog_tests/flow_dissector.c b/tools/testing/selftests/bpf/prog_tests/flow_dissector.c
index 6892b88ae065..dbd20338a667 100644
--- a/tools/testing/selftests/bpf/prog_tests/flow_dissector.c
+++ b/tools/testing/selftests/bpf/prog_tests/flow_dissector.c
@@ -452,10 +452,8 @@ void test_flow_dissector(void)
err = bpf_flow_load(&obj, "./bpf_flow.o", "flow_dissector",
"jmp_table", "last_dissection", &prog_fd, &keys_fd);
- if (err) {
- error_cnt++;
+ if (QCHECK(err))
return;
- }
for (i = 0; i < ARRAY_SIZE(tests); i++) {
struct bpf_flow_keys flow_keys;
diff --git a/tools/testing/selftests/bpf/prog_tests/get_stack_raw_tp.c b/tools/testing/selftests/bpf/prog_tests/get_stack_raw_tp.c
index 3d59b3c841fe..eba9a970703b 100644
--- a/tools/testing/selftests/bpf/prog_tests/get_stack_raw_tp.c
+++ b/tools/testing/selftests/bpf/prog_tests/get_stack_raw_tp.c
@@ -135,10 +135,7 @@ void test_get_stack_raw_tp(void)
exp_cnt -= err;
}
- goto close_prog_noerr;
close_prog:
- error_cnt++;
-close_prog_noerr:
if (!IS_ERR_OR_NULL(link))
bpf_link__destroy(link);
if (!IS_ERR_OR_NULL(pb))
diff --git a/tools/testing/selftests/bpf/prog_tests/global_data.c b/tools/testing/selftests/bpf/prog_tests/global_data.c
index d011079fb0bf..4978cfc5ea25 100644
--- a/tools/testing/selftests/bpf/prog_tests/global_data.c
+++ b/tools/testing/selftests/bpf/prog_tests/global_data.c
@@ -7,10 +7,8 @@ static void test_global_data_number(struct bpf_object *obj, __u32 duration)
uint64_t num;
map_fd = bpf_find_map(__func__, obj, "result_number");
- if (map_fd < 0) {
- error_cnt++;
+ if (QCHECK(map_fd < 0))
return;
- }
struct {
char *name;
@@ -44,10 +42,8 @@ static void test_global_data_string(struct bpf_object *obj, __u32 duration)
char str[32];
map_fd = bpf_find_map(__func__, obj, "result_string");
- if (map_fd < 0) {
- error_cnt++;
+ if (QCHECK(map_fd < 0))
return;
- }
struct {
char *name;
@@ -81,10 +77,8 @@ static void test_global_data_struct(struct bpf_object *obj, __u32 duration)
struct foo val;
map_fd = bpf_find_map(__func__, obj, "result_struct");
- if (map_fd < 0) {
- error_cnt++;
+ if (QCHECK(map_fd < 0))
return;
- }
struct {
char *name;
@@ -112,16 +106,12 @@ static void test_global_data_rdonly(struct bpf_object *obj, __u32 duration)
__u8 *buff;
map = bpf_object__find_map_by_name(obj, "test_glo.rodata");
- if (!map || !bpf_map__is_internal(map)) {
- error_cnt++;
+ if (QCHECK(!map || !bpf_map__is_internal(map)))
return;
- }
map_fd = bpf_map__fd(map);
- if (map_fd < 0) {
- error_cnt++;
+ if (QCHECK(map_fd < 0))
return;
- }
buff = malloc(bpf_map__def(map)->value_size);
if (buff)
diff --git a/tools/testing/selftests/bpf/prog_tests/l4lb_all.c b/tools/testing/selftests/bpf/prog_tests/l4lb_all.c
index 20ddca830e68..5c6de66c1b82 100644
--- a/tools/testing/selftests/bpf/prog_tests/l4lb_all.c
+++ b/tools/testing/selftests/bpf/prog_tests/l4lb_all.c
@@ -30,10 +30,8 @@ static void test_l4lb(const char *file)
u32 *magic = (u32 *)buf;
err = bpf_prog_load(file, BPF_PROG_TYPE_SCHED_CLS, &obj, &prog_fd);
- if (err) {
- error_cnt++;
+ if (QCHECK(err))
return;
- }
map_fd = bpf_find_map(__func__, obj, "vip_map");
if (map_fd < 0)
@@ -72,10 +70,8 @@ static void test_l4lb(const char *file)
bytes += stats[i].bytes;
pkts += stats[i].pkts;
}
- if (bytes != MAGIC_BYTES * NUM_ITER * 2 || pkts != NUM_ITER * 2) {
- error_cnt++;
+ if (QCHECK(bytes != MAGIC_BYTES * NUM_ITER * 2 || pkts != NUM_ITER * 2))
printf("test_l4lb:FAIL:stats %lld %lld\n", bytes, pkts);
- }
out:
bpf_object__close(obj);
}
diff --git a/tools/testing/selftests/bpf/prog_tests/map_lock.c b/tools/testing/selftests/bpf/prog_tests/map_lock.c
index ee99368c595c..c1bddc433a5a 100644
--- a/tools/testing/selftests/bpf/prog_tests/map_lock.c
+++ b/tools/testing/selftests/bpf/prog_tests/map_lock.c
@@ -8,14 +8,12 @@ static void *parallel_map_access(void *arg)
for (i = 0; i < 10000; i++) {
err = bpf_map_lookup_elem_flags(map_fd, &key, vars, BPF_F_LOCK);
- if (err) {
+ if (QCHECK(err)) {
printf("lookup failed\n");
- error_cnt++;
goto out;
}
- if (vars[0] != 0) {
+ if (QCHECK(vars[0] != 0)) {
printf("lookup #%d var[0]=%d\n", i, vars[0]);
- error_cnt++;
goto out;
}
rnd = vars[1];
@@ -24,7 +22,7 @@ static void *parallel_map_access(void *arg)
continue;
printf("lookup #%d var[1]=%d var[%d]=%d\n",
i, rnd, j, vars[j]);
- error_cnt++;
+ QCHECK(vars[j] != rnd);
goto out;
}
}
@@ -42,15 +40,15 @@ void test_map_lock(void)
void *ret;
err = bpf_prog_load(file, BPF_PROG_TYPE_CGROUP_SKB, &obj, &prog_fd);
- if (err) {
+ if (QCHECK(err)) {
printf("test_map_lock:bpf_prog_load errno %d\n", errno);
goto close_prog;
}
map_fd[0] = bpf_find_map(__func__, obj, "hash_map");
- if (map_fd[0] < 0)
+ if (QCHECK(map_fd[0] < 0))
goto close_prog;
map_fd[1] = bpf_find_map(__func__, obj, "array_map");
- if (map_fd[1] < 0)
+ if (QCHECK(map_fd[1] < 0))
goto close_prog;
bpf_map_update_elem(map_fd[0], &key, vars, BPF_F_LOCK);
@@ -67,9 +65,6 @@ void test_map_lock(void)
for (i = 4; i < 6; i++)
assert(pthread_join(thread_id[i], &ret) == 0 &&
ret == (void *)&map_fd[i - 4]);
- goto close_prog_noerr;
close_prog:
- error_cnt++;
-close_prog_noerr:
bpf_object__close(obj);
}
diff --git a/tools/testing/selftests/bpf/prog_tests/pkt_access.c b/tools/testing/selftests/bpf/prog_tests/pkt_access.c
index 4ecfd721a044..345c5201acee 100644
--- a/tools/testing/selftests/bpf/prog_tests/pkt_access.c
+++ b/tools/testing/selftests/bpf/prog_tests/pkt_access.c
@@ -9,10 +9,8 @@ void test_pkt_access(void)
int err, prog_fd;
err = bpf_prog_load(file, BPF_PROG_TYPE_SCHED_CLS, &obj, &prog_fd);
- if (err) {
- error_cnt++;
+ if (QCHECK(err))
return;
- }
err = bpf_prog_test_run(prog_fd, 100000, &pkt_v4, sizeof(pkt_v4),
NULL, NULL, &retval, &duration);
diff --git a/tools/testing/selftests/bpf/prog_tests/pkt_md_access.c b/tools/testing/selftests/bpf/prog_tests/pkt_md_access.c
index ac0d43435806..c0723e306f6f 100644
--- a/tools/testing/selftests/bpf/prog_tests/pkt_md_access.c
+++ b/tools/testing/selftests/bpf/prog_tests/pkt_md_access.c
@@ -9,10 +9,8 @@ void test_pkt_md_access(void)
int err, prog_fd;
err = bpf_prog_load(file, BPF_PROG_TYPE_SCHED_CLS, &obj, &prog_fd);
- if (err) {
- error_cnt++;
+ if (QCHECK(err))
return;
- }
err = bpf_prog_test_run(prog_fd, 10, &pkt_v4, sizeof(pkt_v4),
NULL, NULL, &retval, &duration);
diff --git a/tools/testing/selftests/bpf/prog_tests/queue_stack_map.c b/tools/testing/selftests/bpf/prog_tests/queue_stack_map.c
index e60cd5ff1f55..99e346717767 100644
--- a/tools/testing/selftests/bpf/prog_tests/queue_stack_map.c
+++ b/tools/testing/selftests/bpf/prog_tests/queue_stack_map.c
@@ -27,10 +27,8 @@ static void test_queue_stack_map_by_type(int type)
return;
err = bpf_prog_load(file, BPF_PROG_TYPE_SCHED_CLS, &obj, &prog_fd);
- if (err) {
- error_cnt++;
+ if (QCHECK(err))
return;
- }
map_in_fd = bpf_find_map(__func__, obj, "map_in");
if (map_in_fd < 0)
@@ -43,10 +41,8 @@ static void test_queue_stack_map_by_type(int type)
/* Push 32 elements to the input map */
for (i = 0; i < MAP_SIZE; i++) {
err = bpf_map_update_elem(map_in_fd, NULL, &vals[i], 0);
- if (err) {
- error_cnt++;
+ if (QCHECK(err))
goto out;
- }
}
/* The eBPF program pushes iph.saddr in the output map,
diff --git a/tools/testing/selftests/bpf/prog_tests/reference_tracking.c b/tools/testing/selftests/bpf/prog_tests/reference_tracking.c
index 4a4f428d1a78..cb2fe7b0dc46 100644
--- a/tools/testing/selftests/bpf/prog_tests/reference_tracking.c
+++ b/tools/testing/selftests/bpf/prog_tests/reference_tracking.c
@@ -10,10 +10,8 @@ void test_reference_tracking(void)
int err = 0;
obj = bpf_object__open(file);
- if (IS_ERR(obj)) {
- error_cnt++;
+ if (QCHECK(IS_ERR(obj)))
return;
- }
bpf_object__for_each_program(prog, obj) {
const char *title;
diff --git a/tools/testing/selftests/bpf/prog_tests/spinlock.c b/tools/testing/selftests/bpf/prog_tests/spinlock.c
index 114ebe6a438e..e4294a7fdf1a 100644
--- a/tools/testing/selftests/bpf/prog_tests/spinlock.c
+++ b/tools/testing/selftests/bpf/prog_tests/spinlock.c
@@ -11,7 +11,7 @@ void test_spinlock(void)
void *ret;
err = bpf_prog_load(file, BPF_PROG_TYPE_CGROUP_SKB, &obj, &prog_fd);
- if (err) {
+ if (QCHECK(err)) {
printf("test_spin_lock:bpf_prog_load errno %d\n", errno);
goto close_prog;
}
@@ -21,9 +21,7 @@ void test_spinlock(void)
for (i = 0; i < 4; i++)
assert(pthread_join(thread_id[i], &ret) == 0 &&
ret == (void *)&prog_fd);
- goto close_prog_noerr;
+
close_prog:
- error_cnt++;
-close_prog_noerr:
bpf_object__close(obj);
}
diff --git a/tools/testing/selftests/bpf/prog_tests/stacktrace_map.c b/tools/testing/selftests/bpf/prog_tests/stacktrace_map.c
index fc539335c5b3..708d4cded52a 100644
--- a/tools/testing/selftests/bpf/prog_tests/stacktrace_map.c
+++ b/tools/testing/selftests/bpf/prog_tests/stacktrace_map.c
@@ -26,19 +26,19 @@ void test_stacktrace_map(void)
/* find map fds */
control_map_fd = bpf_find_map(__func__, obj, "control_map");
- if (control_map_fd < 0)
+ if (QCHECK(control_map_fd < 0))
goto disable_pmu;
stackid_hmap_fd = bpf_find_map(__func__, obj, "stackid_hmap");
- if (stackid_hmap_fd < 0)
+ if (QCHECK(stackid_hmap_fd < 0))
goto disable_pmu;
stackmap_fd = bpf_find_map(__func__, obj, "stackmap");
- if (stackmap_fd < 0)
+ if (QCHECK(stackmap_fd < 0))
goto disable_pmu;
stack_amap_fd = bpf_find_map(__func__, obj, "stack_amap");
- if (stack_amap_fd < 0)
+ if (QCHECK(stack_amap_fd < 0))
goto disable_pmu;
/* give some time for bpf program run */
@@ -55,23 +55,20 @@ void test_stacktrace_map(void)
err = compare_map_keys(stackid_hmap_fd, stackmap_fd);
if (CHECK(err, "compare_map_keys stackid_hmap vs. stackmap",
"err %d errno %d\n", err, errno))
- goto disable_pmu_noerr;
+ goto disable_pmu;
err = compare_map_keys(stackmap_fd, stackid_hmap_fd);
if (CHECK(err, "compare_map_keys stackmap vs. stackid_hmap",
"err %d errno %d\n", err, errno))
- goto disable_pmu_noerr;
+ goto disable_pmu;
stack_trace_len = PERF_MAX_STACK_DEPTH * sizeof(__u64);
err = compare_stack_ips(stackmap_fd, stack_amap_fd, stack_trace_len);
if (CHECK(err, "compare_stack_ips stackmap vs. stack_amap",
"err %d errno %d\n", err, errno))
- goto disable_pmu_noerr;
+ goto disable_pmu;
- goto disable_pmu_noerr;
disable_pmu:
- error_cnt++;
-disable_pmu_noerr:
bpf_link__destroy(link);
close_prog:
bpf_object__close(obj);
diff --git a/tools/testing/selftests/bpf/prog_tests/stacktrace_map_raw_tp.c b/tools/testing/selftests/bpf/prog_tests/stacktrace_map_raw_tp.c
index fbfa8e76cf63..eb0208863fa3 100644
--- a/tools/testing/selftests/bpf/prog_tests/stacktrace_map_raw_tp.c
+++ b/tools/testing/selftests/bpf/prog_tests/stacktrace_map_raw_tp.c
@@ -26,15 +26,15 @@ void test_stacktrace_map_raw_tp(void)
/* find map fds */
control_map_fd = bpf_find_map(__func__, obj, "control_map");
- if (control_map_fd < 0)
+ if (QCHECK(control_map_fd < 0))
goto close_prog;
stackid_hmap_fd = bpf_find_map(__func__, obj, "stackid_hmap");
- if (stackid_hmap_fd < 0)
+ if (QCHECK(stackid_hmap_fd < 0))
goto close_prog;
stackmap_fd = bpf_find_map(__func__, obj, "stackmap");
- if (stackmap_fd < 0)
+ if (QCHECK(stackmap_fd < 0))
goto close_prog;
/* give some time for bpf program run */
@@ -58,10 +58,7 @@ void test_stacktrace_map_raw_tp(void)
"err %d errno %d\n", err, errno))
goto close_prog;
- goto close_prog_noerr;
close_prog:
- error_cnt++;
-close_prog_noerr:
if (!IS_ERR_OR_NULL(link))
bpf_link__destroy(link);
bpf_object__close(obj);
diff --git a/tools/testing/selftests/bpf/prog_tests/task_fd_query_rawtp.c b/tools/testing/selftests/bpf/prog_tests/task_fd_query_rawtp.c
index 958a3d88de99..1bdc1d86a50c 100644
--- a/tools/testing/selftests/bpf/prog_tests/task_fd_query_rawtp.c
+++ b/tools/testing/selftests/bpf/prog_tests/task_fd_query_rawtp.c
@@ -70,9 +70,6 @@ void test_task_fd_query_rawtp(void)
if (CHECK(!err, "check_results", "fd_type %d len %u\n", fd_type, len))
goto close_prog;
- goto close_prog_noerr;
close_prog:
- error_cnt++;
-close_prog_noerr:
bpf_object__close(obj);
}
diff --git a/tools/testing/selftests/bpf/prog_tests/task_fd_query_tp.c b/tools/testing/selftests/bpf/prog_tests/task_fd_query_tp.c
index f9b70e81682b..3f131b8fe328 100644
--- a/tools/testing/selftests/bpf/prog_tests/task_fd_query_tp.c
+++ b/tools/testing/selftests/bpf/prog_tests/task_fd_query_tp.c
@@ -62,14 +62,9 @@ static void test_task_fd_query_tp_core(const char *probe_name,
fd_type, buf))
goto close_pmu;
- close(pmu_fd);
- goto close_prog_noerr;
-
close_pmu:
close(pmu_fd);
close_prog:
- error_cnt++;
-close_prog_noerr:
bpf_object__close(obj);
}
diff --git a/tools/testing/selftests/bpf/prog_tests/tcp_estats.c b/tools/testing/selftests/bpf/prog_tests/tcp_estats.c
index bb8759d69099..594307dffd13 100644
--- a/tools/testing/selftests/bpf/prog_tests/tcp_estats.c
+++ b/tools/testing/selftests/bpf/prog_tests/tcp_estats.c
@@ -10,10 +10,8 @@ void test_tcp_estats(void)
err = bpf_prog_load(file, BPF_PROG_TYPE_TRACEPOINT, &obj, &prog_fd);
CHECK(err, "", "err %d errno %d\n", err, errno);
- if (err) {
- error_cnt++;
+ if (err)
return;
- }
bpf_object__close(obj);
}
diff --git a/tools/testing/selftests/bpf/prog_tests/xdp.c b/tools/testing/selftests/bpf/prog_tests/xdp.c
index a74167289545..a5da2bb221d4 100644
--- a/tools/testing/selftests/bpf/prog_tests/xdp.c
+++ b/tools/testing/selftests/bpf/prog_tests/xdp.c
@@ -16,10 +16,8 @@ void test_xdp(void)
int err, prog_fd, map_fd;
err = bpf_prog_load(file, BPF_PROG_TYPE_XDP, &obj, &prog_fd);
- if (err) {
- error_cnt++;
+ if (QCHECK(err))
return;
- }
map_fd = bpf_find_map(__func__, obj, "vip2tnl");
if (map_fd < 0)
diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_adjust_tail.c b/tools/testing/selftests/bpf/prog_tests/xdp_adjust_tail.c
index 922aa0a19764..115a2b883d1e 100644
--- a/tools/testing/selftests/bpf/prog_tests/xdp_adjust_tail.c
+++ b/tools/testing/selftests/bpf/prog_tests/xdp_adjust_tail.c
@@ -10,10 +10,8 @@ void test_xdp_adjust_tail(void)
int err, prog_fd;
err = bpf_prog_load(file, BPF_PROG_TYPE_XDP, &obj, &prog_fd);
- if (err) {
- error_cnt++;
+ if (QCHECK(err))
return;
- }
err = bpf_prog_test_run(prog_fd, 1, &pkt_v4, sizeof(pkt_v4),
buf, &size, &retval, &duration);
diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_noinline.c b/tools/testing/selftests/bpf/prog_tests/xdp_noinline.c
index 15f7c272edb0..edfff407ac69 100644
--- a/tools/testing/selftests/bpf/prog_tests/xdp_noinline.c
+++ b/tools/testing/selftests/bpf/prog_tests/xdp_noinline.c
@@ -31,10 +31,8 @@ void test_xdp_noinline(void)
u32 *magic = (u32 *)buf;
err = bpf_prog_load(file, BPF_PROG_TYPE_XDP, &obj, &prog_fd);
- if (err) {
- error_cnt++;
+ if (QCHECK(err))
return;
- }
map_fd = bpf_find_map(__func__, obj, "vip_map");
if (map_fd < 0)
@@ -73,8 +71,7 @@ void test_xdp_noinline(void)
bytes += stats[i].bytes;
pkts += stats[i].pkts;
}
- if (bytes != MAGIC_BYTES * NUM_ITER * 2 || pkts != NUM_ITER * 2) {
- error_cnt++;
+ if (QCHECK(bytes != MAGIC_BYTES * NUM_ITER * 2 || pkts != NUM_ITER * 2)) {
printf("test_xdp_noinline:FAIL:stats %lld %lld\n",
bytes, pkts);
}
diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index e545dfb55872..e5892cb60eca 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -8,14 +8,12 @@
/* defined in test_progs.h */
struct test_env env;
-int error_cnt, pass_cnt;
struct prog_test_def {
const char *test_name;
int test_num;
void (*run_test)(void);
bool force_log;
- int pass_cnt;
int error_cnt;
int skip_cnt;
bool tested;
@@ -24,7 +22,6 @@ struct prog_test_def {
int subtest_num;
/* store counts before subtest started */
- int old_pass_cnt;
int old_error_cnt;
};
@@ -68,7 +65,7 @@ static void skip_account(void)
void test__end_subtest()
{
struct prog_test_def *test = env.test;
- int sub_error_cnt = error_cnt - test->old_error_cnt;
+ int sub_error_cnt = test->error_cnt - test->old_error_cnt;
if (sub_error_cnt)
env.fail_cnt++;
@@ -105,8 +102,7 @@ bool test__start_subtest(const char *name)
return false;
test->subtest_name = name;
- env.test->old_pass_cnt = pass_cnt;
- env.test->old_error_cnt = error_cnt;
+ env.test->old_error_cnt = env.test->error_cnt;
return true;
}
@@ -120,6 +116,11 @@ void test__skip(void)
env.test->skip_cnt++;
}
+void test__fail(void)
+{
+ env.test->error_cnt++;
+}
+
struct ipv4_packet pkt_v4 = {
.eth.h_proto = __bpf_constant_htons(ETH_P_IP),
.iph.ihl = 5,
@@ -144,7 +145,7 @@ int bpf_find_map(const char *test, struct bpf_object *obj, const char *name)
map = bpf_object__find_map_by_name(obj, name);
if (!map) {
printf("%s:FAIL:map '%s' not found\n", test, name);
- error_cnt++;
+ test__fail();
return -1;
}
return bpf_map__fd(map);
@@ -503,8 +504,6 @@ int main(int argc, char **argv)
stdio_hijack();
for (i = 0; i < prog_test_cnt; i++) {
struct prog_test_def *test = &prog_test_defs[i];
- int old_pass_cnt = pass_cnt;
- int old_error_cnt = error_cnt;
env.test = test;
test->test_num = i + 1;
@@ -519,8 +518,6 @@ int main(int argc, char **argv)
test__end_subtest();
test->tested = true;
- test->pass_cnt = pass_cnt - old_pass_cnt;
- test->error_cnt = error_cnt - old_error_cnt;
if (test->error_cnt)
env.fail_cnt++;
else
@@ -540,5 +537,5 @@ int main(int argc, char **argv)
free(env.test_selector.num_set);
free(env.subtest_selector.num_set);
- return error_cnt ? EXIT_FAILURE : EXIT_SUCCESS;
+ return env.fail_cnt ? EXIT_FAILURE : EXIT_SUCCESS;
}
diff --git a/tools/testing/selftests/bpf/test_progs.h b/tools/testing/selftests/bpf/test_progs.h
index 9defd35cb6c0..a74c1ca66bba 100644
--- a/tools/testing/selftests/bpf/test_progs.h
+++ b/tools/testing/selftests/bpf/test_progs.h
@@ -38,8 +38,6 @@ typedef __u16 __sum16;
#include "trace_helpers.h"
#include "flow_dissector_load.h"
-struct prog_test_def;
-
struct test_selector {
const char *name;
bool *num_set;
@@ -67,13 +65,12 @@ struct test_env {
int skip_cnt; /* skipped tests */
};
-extern int error_cnt;
-extern int pass_cnt;
extern struct test_env env;
extern void test__force_log();
extern bool test__start_subtest(const char *name);
extern void test__skip(void);
+extern void test__fail(void);
#define MAGIC_BYTES 123
@@ -96,17 +93,25 @@ extern struct ipv6_packet pkt_v6;
#define _CHECK(condition, tag, duration, format...) ({ \
int __ret = !!(condition); \
if (__ret) { \
- error_cnt++; \
+ test__fail(); \
printf("%s:FAIL:%s ", __func__, tag); \
printf(format); \
} else { \
- pass_cnt++; \
printf("%s:PASS:%s %d nsec\n", \
__func__, tag, duration); \
} \
__ret; \
})
+#define QCHECK(condition) ({ \
+ int __ret = !!(condition); \
+ if (__ret) { \
+ test__fail(); \
+ printf("%s:FAIL:%d ", __func__, __LINE__); \
+ } \
+ __ret; \
+})
+
#define CHECK(condition, tag, format...) \
_CHECK(condition, tag, duration, format)
#define CHECK_ATTR(condition, tag, format...) \
--
2.23.0.rc1.153.gdeed80330f-goog
^ permalink raw reply related
* [PATCH bpf-next v2 3/4] selftests/bpf: test_progs: remove asserts from subtests
From: Stanislav Fomichev @ 2019-08-19 19:17 UTC (permalink / raw)
To: netdev, bpf; +Cc: davem, ast, daniel, Stanislav Fomichev, Andrii Nakryiko
In-Reply-To: <20190819191752.241637-1-sdf@google.com>
Otherwise they can bring the whole process down.
Cc: Andrii Nakryiko <andriin@fb.com>
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
.../selftests/bpf/prog_tests/bpf_obj_id.c | 19 +++++++++++-------
.../selftests/bpf/prog_tests/map_lock.c | 20 +++++++++++--------
.../selftests/bpf/prog_tests/spinlock.c | 12 ++++++-----
.../bpf/prog_tests/stacktrace_build_id.c | 7 ++++---
.../bpf/prog_tests/stacktrace_build_id_nmi.c | 7 ++++---
5 files changed, 39 insertions(+), 26 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_obj_id.c b/tools/testing/selftests/bpf/prog_tests/bpf_obj_id.c
index b9d0cd312839..acc2fc046b01 100644
--- a/tools/testing/selftests/bpf/prog_tests/bpf_obj_id.c
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_obj_id.c
@@ -48,15 +48,17 @@ void test_bpf_obj_id(void)
/* test_obj_id.o is a dumb prog. It should never fail
* to load.
*/
- QCHECK(err);
- assert(!err);
+ if (QCHECK(err))
+ continue;
/* Insert a magic value to the map */
map_fds[i] = bpf_find_map(__func__, objs[i], "test_map_id");
- assert(map_fds[i] >= 0);
+ if (QCHECK(map_fds[i] < 0))
+ goto done;
err = bpf_map_update_elem(map_fds[i], &array_key,
&array_magic_value, 0);
- assert(!err);
+ if (QCHECK(err))
+ goto done;
/* Check getting map info */
info_len = sizeof(struct bpf_map_info) * 2;
@@ -95,9 +97,11 @@ void test_bpf_obj_id(void)
prog_infos[i].map_ids = ptr_to_u64(map_ids + i);
prog_infos[i].nr_map_ids = 2;
err = clock_gettime(CLOCK_REALTIME, &real_time_ts);
- assert(!err);
+ if (QCHECK(err))
+ goto done;
err = clock_gettime(CLOCK_BOOTTIME, &boot_time_ts);
- assert(!err);
+ if (QCHECK(err))
+ goto done;
err = bpf_obj_get_info_by_fd(prog_fds[i], &prog_infos[i],
&info_len);
load_time = (real_time_ts.tv_sec - boot_time_ts.tv_sec)
@@ -223,7 +227,8 @@ void test_bpf_obj_id(void)
nr_id_found++;
err = bpf_map_lookup_elem(map_fd, &array_key, &array_value);
- assert(!err);
+ if (QCHECK(err))
+ goto done;
err = bpf_obj_get_info_by_fd(map_fd, &map_info, &info_len);
CHECK(err || info_len != sizeof(struct bpf_map_info) ||
diff --git a/tools/testing/selftests/bpf/prog_tests/map_lock.c b/tools/testing/selftests/bpf/prog_tests/map_lock.c
index c1bddc433a5a..7a12129def9a 100644
--- a/tools/testing/selftests/bpf/prog_tests/map_lock.c
+++ b/tools/testing/selftests/bpf/prog_tests/map_lock.c
@@ -54,17 +54,21 @@ void test_map_lock(void)
bpf_map_update_elem(map_fd[0], &key, vars, BPF_F_LOCK);
for (i = 0; i < 4; i++)
- assert(pthread_create(&thread_id[i], NULL,
- &spin_lock_thread, &prog_fd) == 0);
+ if (QCHECK(pthread_create(&thread_id[i], NULL,
+ &spin_lock_thread, &prog_fd)))
+ goto close_prog;
for (i = 4; i < 6; i++)
- assert(pthread_create(&thread_id[i], NULL,
- ¶llel_map_access, &map_fd[i - 4]) == 0);
+ if (QCHECK(pthread_create(&thread_id[i], NULL,
+ ¶llel_map_access, &map_fd[i - 4])))
+ goto close_prog;
for (i = 0; i < 4; i++)
- assert(pthread_join(thread_id[i], &ret) == 0 &&
- ret == (void *)&prog_fd);
+ if (QCHECK(pthread_join(thread_id[i], &ret) ||
+ ret != (void *)&prog_fd))
+ goto close_prog;
for (i = 4; i < 6; i++)
- assert(pthread_join(thread_id[i], &ret) == 0 &&
- ret == (void *)&map_fd[i - 4]);
+ if (QCHECK(pthread_join(thread_id[i], &ret) ||
+ ret != (void *)&map_fd[i - 4]))
+ goto close_prog;
close_prog:
bpf_object__close(obj);
}
diff --git a/tools/testing/selftests/bpf/prog_tests/spinlock.c b/tools/testing/selftests/bpf/prog_tests/spinlock.c
index e4294a7fdf1a..00b4ed1734e0 100644
--- a/tools/testing/selftests/bpf/prog_tests/spinlock.c
+++ b/tools/testing/selftests/bpf/prog_tests/spinlock.c
@@ -16,12 +16,14 @@ void test_spinlock(void)
goto close_prog;
}
for (i = 0; i < 4; i++)
- assert(pthread_create(&thread_id[i], NULL,
- &spin_lock_thread, &prog_fd) == 0);
- for (i = 0; i < 4; i++)
- assert(pthread_join(thread_id[i], &ret) == 0 &&
- ret == (void *)&prog_fd);
+ if (QCHECK(pthread_create(&thread_id[i], NULL,
+ &spin_lock_thread, &prog_fd)))
+ goto close_prog;
+ for (i = 0; i < 4; i++)
+ if (QCHECK(pthread_join(thread_id[i], &ret) ||
+ ret != (void *)&prog_fd))
+ goto close_prog;
close_prog:
bpf_object__close(obj);
}
diff --git a/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id.c b/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id.c
index ac44fda84833..552e07e7800c 100644
--- a/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id.c
+++ b/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id.c
@@ -51,9 +51,10 @@ void test_stacktrace_build_id(void)
"err %d errno %d\n", err, errno))
goto disable_pmu;
- assert(system("dd if=/dev/urandom of=/dev/zero count=4 2> /dev/null")
- == 0);
- assert(system("./urandom_read") == 0);
+ if (QCHECK(system("dd if=/dev/urandom of=/dev/zero count=4 2> /dev/null")))
+ goto disable_pmu;
+ if (QCHECK(system("./urandom_read")))
+ goto disable_pmu;
/* disable stack trace collection */
key = 0;
val = 1;
diff --git a/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c b/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c
index 9557b7dfb782..1553c848edc5 100644
--- a/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c
+++ b/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c
@@ -82,9 +82,10 @@ void test_stacktrace_build_id_nmi(void)
"err %d errno %d\n", err, errno))
goto disable_pmu;
- assert(system("dd if=/dev/urandom of=/dev/zero count=4 2> /dev/null")
- == 0);
- assert(system("taskset 0x1 ./urandom_read 100000") == 0);
+ if (QCHECK(system("dd if=/dev/urandom of=/dev/zero count=4 2> /dev/null")))
+ goto disable_pmu;
+ if (QCHECK(system("taskset 0x1 ./urandom_read 100000")))
+ goto disable_pmu;
/* disable stack trace collection */
key = 0;
val = 1;
--
2.23.0.rc1.153.gdeed80330f-goog
^ 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